[U-Boot] [PATCH 1/4] fs: cbfs: remove wrong header validation

From: Christian Gmeiner christian.gmeiner@gmail.com
cbfs_fileheader.len indicates the content size of the file in the cbfs, and it has nothing to do with cbfs_fileheader.offset which is the starting address of the file in the cbfs.
Remove such check in file_cbfs_next_file(). Before this change 'cbfsinit' failed with 'Bad CBFS file'. After this change all cbfs commands are working as expected.
Signed-off-by: Christian Gmeiner christian.gmeiner@gmail.com [bmeng: keep the necessary header sanity check] Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
fs/cbfs/cbfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index 0dce639..e943325 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -96,8 +96,7 @@ 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) { + if (header.offset < sizeof(struct cbfs_fileheader)) { file_cbfs_result = CBFS_BAD_FILE; return -1; }

The call to file_cbfs_fill_cache() is given with the parameter 'start' pointing to the offset by the CBFS base address, but with the parameter 'size' that equals to the whole CBFS size. During CBFS walking through, it checks files one by one and after it pass over the end of the CBFS which is 4GiB boundary it tries to check files from address 0 and so on, until the overall size the codes checked hits to the given 'size'.
Fix this by passing 'start' pointing to the CBFS base address.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
fs/cbfs/cbfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index e943325..7b2513c 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -189,8 +189,8 @@ void file_cbfs_init(uintptr_t end_of_rom)
start_of_rom = (u8 *)(end_of_rom + 1 - cbfs_header.rom_size);
- file_cbfs_fill_cache(start_of_rom + cbfs_header.offset, - cbfs_header.rom_size, cbfs_header.align); + file_cbfs_fill_cache(start_of_rom, cbfs_header.rom_size, + cbfs_header.align); if (file_cbfs_result == CBFS_SUCCESS) initialized = 1; }

On Sat, 22 Dec 2018 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
The call to file_cbfs_fill_cache() is given with the parameter 'start' pointing to the offset by the CBFS base address, but with the parameter 'size' that equals to the whole CBFS size. During CBFS walking through, it checks files one by one and after it pass over the end of the CBFS which is 4GiB boundary it tries to check files from address 0 and so on, until the overall size the codes checked hits to the given 'size'.
Fix this by passing 'start' pointing to the CBFS base address.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
fs/cbfs/cbfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Dec 29, 2018 at 9:40 PM Simon Glass sjg@chromium.org wrote:
On Sat, 22 Dec 2018 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
The call to file_cbfs_fill_cache() is given with the parameter 'start' pointing to the offset by the CBFS base address, but with the parameter 'size' that equals to the whole CBFS size. During CBFS walking through, it checks files one by one and after it pass over the end of the CBFS which is 4GiB boundary it tries to check files from address 0 and so on, until the overall size the codes checked hits to the given 'size'.
Fix this by passing 'start' pointing to the CBFS base address.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
fs/cbfs/cbfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

At present there are 2 macros that are named as CBFS_COMPONENT_xxx. Change them to CBFS_TYPE_xxx for consistency.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
cmd/cbfs.c | 4 ++-- include/cbfs.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/cmd/cbfs.c b/cmd/cbfs.c index ece790e..4d3e006 100644 --- a/cmd/cbfs.c +++ b/cmd/cbfs.c @@ -136,10 +136,10 @@ static int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, case CBFS_TYPE_MICROCODE: type_name = "microcode"; break; - case CBFS_COMPONENT_CMOS_DEFAULT: + case CBFS_TYPE_CMOS_DEFAULT: type_name = "cmos default"; break; - case CBFS_COMPONENT_CMOS_LAYOUT: + case CBFS_TYPE_CMOS_LAYOUT: type_name = "cmos layout"; break; case -1: diff --git a/include/cbfs.h b/include/cbfs.h index 1b88ec0..dd4b574 100644 --- a/include/cbfs.h +++ b/include/cbfs.h @@ -26,8 +26,8 @@ enum cbfs_filetype { CBFS_TYPE_VSA = 0x51, CBFS_TYPE_MBI = 0x52, CBFS_TYPE_MICROCODE = 0x53, - CBFS_COMPONENT_CMOS_DEFAULT = 0xaa, - CBFS_COMPONENT_CMOS_LAYOUT = 0x01aa + CBFS_TYPE_CMOS_DEFAULT = 0xaa, + CBFS_TYPE_CMOS_LAYOUT = 0x01aa };
struct cbfs_header {

On Sat, 22 Dec 2018 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
At present there are 2 macros that are named as CBFS_COMPONENT_xxx. Change them to CBFS_TYPE_xxx for consistency.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
cmd/cbfs.c | 4 ++-- include/cbfs.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Dec 29, 2018 at 9:40 PM Simon Glass sjg@chromium.org wrote:
On Sat, 22 Dec 2018 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
At present there are 2 macros that are named as CBFS_COMPONENT_xxx. Change them to CBFS_TYPE_xxx for consistency.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
cmd/cbfs.c | 4 ++-- include/cbfs.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

Current CBFS component type list is incomplete. Add missing ones.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
cmd/cbfs.c | 30 ++++++++++++++++++++++++++++++ include/cbfs.h | 10 ++++++++++ 2 files changed, 40 insertions(+)
diff --git a/cmd/cbfs.c b/cmd/cbfs.c index 4d3e006..c118a95 100644 --- a/cmd/cbfs.c +++ b/cmd/cbfs.c @@ -112,12 +112,21 @@ static int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, printf(" %8d", file_cbfs_size(file));
switch (type) { + case CBFS_TYPE_BOOTBLOCK: + type_name = "bootblock"; + break; + case CBFS_TYPE_CBFSHEADER: + type_name = "cbfs header"; + break; case CBFS_TYPE_STAGE: type_name = "stage"; break; case CBFS_TYPE_PAYLOAD: type_name = "payload"; break; + case CBFS_TYPE_FIT: + type_name = "fit"; + break; case CBFS_TYPE_OPTIONROM: type_name = "option rom"; break; @@ -136,9 +145,30 @@ static int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, case CBFS_TYPE_MICROCODE: type_name = "microcode"; break; + case CBFS_TYPE_FSP: + type_name = "fsp"; + break; + case CBFS_TYPE_MRC: + type_name = "mrc"; + break; + case CBFS_TYPE_MMA: + type_name = "mma"; + break; + case CBFS_TYPE_EFI: + type_name = "efi"; + break; + case CBFS_TYPE_STRUCT: + type_name = "struct"; + break; case CBFS_TYPE_CMOS_DEFAULT: type_name = "cmos default"; break; + case CBFS_TYPE_SPD: + type_name = "spd"; + break; + case CBFS_TYPE_MRC_CACHE: + type_name = "mrc cache"; + break; case CBFS_TYPE_CMOS_LAYOUT: type_name = "cmos layout"; break; diff --git a/include/cbfs.h b/include/cbfs.h index dd4b574..bd1bf75 100644 --- a/include/cbfs.h +++ b/include/cbfs.h @@ -18,15 +18,25 @@ enum cbfs_result { };
enum cbfs_filetype { + CBFS_TYPE_BOOTBLOCK = 0x01, + CBFS_TYPE_CBFSHEADER = 0x02, CBFS_TYPE_STAGE = 0x10, CBFS_TYPE_PAYLOAD = 0x20, + CBFS_TYPE_FIT = 0x21, CBFS_TYPE_OPTIONROM = 0x30, CBFS_TYPE_BOOTSPLASH = 0x40, CBFS_TYPE_RAW = 0x50, CBFS_TYPE_VSA = 0x51, CBFS_TYPE_MBI = 0x52, CBFS_TYPE_MICROCODE = 0x53, + CBFS_TYPE_FSP = 0x60, + CBFS_TYPE_MRC = 0x61, + CBFS_TYPE_MMA = 0x62, + CBFS_TYPE_EFI = 0x63, + CBFS_TYPE_STRUCT = 0x70, CBFS_TYPE_CMOS_DEFAULT = 0xaa, + CBFS_TYPE_SPD = 0xab, + CBFS_TYPE_MRC_CACHE = 0xac, CBFS_TYPE_CMOS_LAYOUT = 0x01aa };

On Sat, 22 Dec 2018 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Current CBFS component type list is incomplete. Add missing ones.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
cmd/cbfs.c | 30 ++++++++++++++++++++++++++++++ include/cbfs.h | 10 ++++++++++ 2 files changed, 40 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Dec 29, 2018 at 9:40 PM Simon Glass sjg@chromium.org wrote:
On Sat, 22 Dec 2018 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
Current CBFS component type list is incomplete. Add missing ones.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
cmd/cbfs.c | 30 ++++++++++++++++++++++++++++++ include/cbfs.h | 10 ++++++++++ 2 files changed, 40 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

On Sat, 22 Dec 2018 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
From: Christian Gmeiner christian.gmeiner@gmail.com
cbfs_fileheader.len indicates the content size of the file in the cbfs, and it has nothing to do with cbfs_fileheader.offset which is the starting address of the file in the cbfs.
Remove such check in file_cbfs_next_file(). Before this change 'cbfsinit' failed with 'Bad CBFS file'. After this change all cbfs commands are working as expected.
Signed-off-by: Christian Gmeiner christian.gmeiner@gmail.com [bmeng: keep the necessary header sanity check] Signed-off-by: Bin Meng bmeng.cn@gmail.com
fs/cbfs/cbfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Dec 29, 2018 at 9:40 PM Simon Glass sjg@chromium.org wrote:
On Sat, 22 Dec 2018 at 02:50, Bin Meng bmeng.cn@gmail.com wrote:
From: Christian Gmeiner christian.gmeiner@gmail.com
cbfs_fileheader.len indicates the content size of the file in the cbfs, and it has nothing to do with cbfs_fileheader.offset which is the starting address of the file in the cbfs.
Remove such check in file_cbfs_next_file(). Before this change 'cbfsinit' failed with 'Bad CBFS file'. After this change all cbfs commands are working as expected.
Signed-off-by: Christian Gmeiner christian.gmeiner@gmail.com [bmeng: keep the necessary header sanity check] Signed-off-by: Bin Meng bmeng.cn@gmail.com
fs/cbfs/cbfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!
participants (2)
-
Bin Meng
-
Simon Glass