On 15.02.18 07:40, Andre Heider wrote:
This fixes walking the cbfs file list because the bound checks do not apply to header components.
Output of coreboot's cbfstool: Name Offset Type Size Comp cbfs master header 0x0 cbfs header 32 none fallback/romstage 0x80 stage 21344 none fallback/ramstage 0x5440 stage 36848 none config 0xe480 raw 310 none revision 0xe600 raw 575 none fallback/bl31 0xe880 payload 15931 none fallback/payload 0x12700 payload 205449 none (empty) 0x44a00 null 111768 none header pointer 0x5fec0 cbfs header 4 none
Output of u-boot's cbfsls: size type name ------------------------------------------ 32 cbfs header cbfs master header 21344 stage fallback/romstage 36848 stage fallback/ramstage 310 raw config 575 raw revision 15931 payload fallback/bl31 205449 payload fallback/payload 111768 null (empty) 4 cbfs header header pointer
I don't see a before/after comparison? What output exactly did get fixed?
I don't quite understand what case exactly this fixes. The bounds check seems to try to find out whether the header is within the master header range, right?
So doesn't this just mean we're reading beyond the expected fs size?
Alex
Signed-off-by: Andre Heider a.heider@gmail.com
cmd/cbfs.c | 3 +++ fs/cbfs/cbfs.c | 10 ++++++---- include/cbfs.h | 1 + 3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/cmd/cbfs.c b/cmd/cbfs.c index 736f8c4527..f5ad04c45a 100644 --- a/cmd/cbfs.c +++ b/cmd/cbfs.c @@ -113,6 +113,9 @@ static int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, printf(" %8d", file_cbfs_size(file));
switch (type) {
case CBFS_COMPONENT_CBFSHEADER:type_name = "cbfs header";break;diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index 46da8f134f..389f60702b 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -97,10 +97,12 @@ static int file_cbfs_next_file(u8 *start, u32 size, u32 align, }
swap_file_header(&header, fileHeader);
if (header.offset < sizeof(struct cbfs_fileheader) ||header.offset > header.len) {file_cbfs_result = CBFS_BAD_FILE;return -1;
if (header.type != CBFS_COMPONENT_CBFSHEADER) {if (header.offset < sizeof(struct cbfs_fileheader) ||header.offset > header.len) {file_cbfs_result = CBFS_BAD_FILE;return -1;}diff --git a/include/cbfs.h b/include/cbfs.h index f50280107b..d5d9d8ce97 100644 --- a/include/cbfs.h +++ b/include/cbfs.h @@ -19,6 +19,7 @@ enum cbfs_result { };
enum cbfs_filetype {
- CBFS_COMPONENT_CBFSHEADER = 0x02, CBFS_TYPE_STAGE = 0x10, CBFS_TYPE_PAYLOAD = 0x20, CBFS_TYPE_OPTIONROM = 0x30,