Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58764 )
Change subject: amdfwtool: Pack out-of-bounds check into a function and move ......................................................................
amdfwtool: Pack out-of-bounds check into a function and move
Need to check the FWs number limit several times. So pack the duplicated steps into a function. And do it before access the new entry.
Change-Id: I71117d1c817c0b6ddaea4ea47aea91672cc6d55a Signed-off-by: Zheng Bao fishbaozi@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/58764 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M util/amdfwtool/amdfwtool.c 1 file changed, 15 insertions(+), 13 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 049755b..b10315e 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -354,6 +354,16 @@ #define BUFF_TO_RUN(ctx, ptr) RUN_OFFSET((ctx), ((char *)(ptr) - (ctx).rom)) #define BUFF_ROOM(ctx) ((ctx).rom_size - (ctx).current)
+void assert_fw_entry(uint32_t count, uint32_t max, context *ctx) +{ + if (count >= max) { + fprintf(stderr, "Error: BIOS entries (%d) exceeds max allowed items " + "(%d)\n", count, max); + free(ctx->rom); + exit(1); + } +} + static void *new_psp_dir(context *ctx, int multi) { void *ptr; @@ -653,6 +663,8 @@ if (!(fw_table[i].level & level)) continue;
+ assert_fw_entry(count, MAX_PSP_ENTRIES, ctx); + if (fw_table[i].type == AMD_TOKEN_UNLOCK) { if (!fw_table[i].other) continue; @@ -719,6 +731,7 @@ }
if (pspdir2) { + assert_fw_entry(count, MAX_PSP_ENTRIES, ctx); pspdir->entries[count].type = AMD_FW_L2_PTR; pspdir->entries[count].subprog = 0; pspdir->entries[count].rsvd = 0; @@ -730,12 +743,6 @@ count++; }
- if (count > MAX_PSP_ENTRIES) { - fprintf(stderr, "Error: PSP entries exceed max allowed items\n"); - free(ctx->rom); - exit(1); - } - fill_dir_header(pspdir, count, cookie, ctx); }
@@ -888,6 +895,7 @@ if (fw_table[i].type == AMD_BIOS_PSP_SHARED_MEM && (!fw_table[i].dest || !fw_table[i].size)) continue; + assert_fw_entry(count, MAX_BIOS_ENTRIES, ctx);
biosdir->entries[count].type = fw_table[i].type; biosdir->entries[count].region_type = fw_table[i].region_type; @@ -977,6 +985,7 @@ }
if (biosdir2) { + assert_fw_entry(count, MAX_BIOS_ENTRIES, ctx); biosdir->entries[count].type = AMD_BIOS_L2_PTR; biosdir->entries[count].region_type = 0; biosdir->entries[count].size = @@ -994,13 +1003,6 @@ count++; }
- if (count > MAX_BIOS_ENTRIES) { - fprintf(stderr, "Error: BIOS entries (%d) exceeds max allowed items " - "(%d)\n", count, MAX_BIOS_ENTRIES); - free(ctx->rom); - exit(1); - } - fill_dir_header(biosdir, count, cookie, ctx); }