Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/34846 )
Change subject: internal: Fix board vendor and model memory leaks ......................................................................
internal: Fix board vendor and model memory leaks
The board vendor and model are sometimes specified as arguments during an internal flash, so make sure they are freed at the end of initialization.
Change-Id: I9f43708f3b075896be67acec114bc6f390f8c6ca Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1230664, 1230665 --- M internal.c 1 file changed, 30 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/34846/1
diff --git a/internal.c b/internal.c index 12c0ba3..112702e 100644 --- a/internal.c +++ b/internal.c @@ -210,8 +210,10 @@ } free(arg);
- if (rget_io_perms()) - return 1; + if (rget_io_perms()) { + ret = 1; + goto cleanup; + }
/* Default to Parallel/LPC/FWH flash devices. If a known host controller * is found, the host controller init routine sets the @@ -219,17 +221,22 @@ */ internal_buses_supported = BUS_NONSPI;
- if (try_mtd() == 0) - return 0; + if (try_mtd() == 0) { + ret = 0; + goto cleanup; + }
/* Initialize PCI access for flash enables */ - if (pci_init_common() != 0) - return 1; + if (pci_init_common() != 0) { + ret = 1; + goto cleanup; + }
if (processor_flash_enable()) { msg_perr("Processor detection/init failed.\n" "Aborting.\n"); - return 1; + ret = 1; + goto cleanup; }
#if IS_X86 @@ -238,8 +245,10 @@ msg_pwarn("Warning: The mainboard IDs set by -p internal:mainboard (%s:%s) do not\n" " match the current coreboot IDs of the mainboard (%s:%s).\n", board_vendor, board_model, cb_vendor, cb_model); - if (!force_boardmismatch) - return 1; + if (!force_boardmismatch) { + ret = 1; + goto cleanup; + } msg_pinfo("Continuing anyway.\n"); } } @@ -281,8 +290,9 @@ if (ret == -2) { msg_perr("WARNING: No chipset found. Flash detection " "will most likely fail.\n"); - } else if (ret == ERROR_FATAL) - return ret; + } else if (ret == ERROR_FATAL) { + goto cleanup; + }
#if IS_X86 /* Probe unconditionally for ITE Super I/O chips. This enables LPC->SPI translation on IT87* and @@ -291,7 +301,8 @@
if (board_flash_enable(board_vendor, board_model, cb_vendor, cb_model)) { msg_perr("Aborting to be safe.\n"); - return 1; + ret = 1; + goto cleanup; } #endif
@@ -325,7 +336,13 @@ "========================================================================\n"); }
- return 0; + ret = 0; + +cleanup: + free(board_vendor); + free(board_model); + + return ret; }
static void internal_chip_writeb(const struct flashctx *flash, uint8_t val,
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/34846
to look at the new patch set (#2).
Change subject: internal: Fix board vendor and model memory leaks ......................................................................
internal: Fix board vendor and model memory leaks
The board vendor and model are sometimes specified as arguments during an internal flash, so make sure they are freed at the end of initialization.
Change-Id: I9f43708f3b075896be67acec114bc6f390f8c6ca Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1230664, 1230665 --- M board_enable.c M internal.c M programmer.h 3 files changed, 34 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/34846/2
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/34846
to look at the new patch set (#3).
Change subject: internal: Fix board vendor and model memory leaks ......................................................................
internal: Fix board vendor and model memory leaks
The board vendor and model are sometimes specified as arguments during an internal flash, so make sure they are freed at the end of initialization.
Change-Id: I9f43708f3b075896be67acec114bc6f390f8c6ca Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1230664, 1230665 --- M board_enable.c M internal.c M programmer.h 3 files changed, 34 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/34846/3
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34846 )
Change subject: internal: Fix board vendor and model memory leaks ......................................................................
Patch Set 3: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/34846 )
Change subject: internal: Fix board vendor and model memory leaks ......................................................................
internal: Fix board vendor and model memory leaks
The board vendor and model are sometimes specified as arguments during an internal flash, so make sure they are freed at the end of initialization.
Change-Id: I9f43708f3b075896be67acec114bc6f390f8c6ca Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1230664, 1230665 Reviewed-on: https://review.coreboot.org/c/flashrom/+/34846 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: David Hendricks david.hendricks@gmail.com --- M board_enable.c M internal.c M programmer.h 3 files changed, 34 insertions(+), 17 deletions(-)
Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved
diff --git a/board_enable.c b/board_enable.c index 74612f5..042ce12 100644 --- a/board_enable.c +++ b/board_enable.c @@ -2516,7 +2516,7 @@ * Parameters vendor and model will be overwritten. Returns 0 on success. * Note: strtok modifies the original string, so we work on a copy and allocate memory for the results. */ -int board_parse_parameter(const char *boardstring, const char **vendor, const char **model) +int board_parse_parameter(const char *boardstring, char **vendor, char **model) { /* strtok may modify the original string. */ char *tempstr = strdup(boardstring); diff --git a/internal.c b/internal.c index 12c0ba3..44570a5 100644 --- a/internal.c +++ b/internal.c @@ -145,8 +145,8 @@ int ret = 0; int force_laptop = 0; int not_a_laptop = 0; - const char *board_vendor = NULL; - const char *board_model = NULL; + char *board_vendor = NULL; + char *board_model = NULL; #if IS_X86 const char *cb_vendor = NULL; const char *cb_model = NULL; @@ -210,8 +210,10 @@ } free(arg);
- if (rget_io_perms()) - return 1; + if (rget_io_perms()) { + ret = 1; + goto internal_init_exit; + }
/* Default to Parallel/LPC/FWH flash devices. If a known host controller * is found, the host controller init routine sets the @@ -219,17 +221,22 @@ */ internal_buses_supported = BUS_NONSPI;
- if (try_mtd() == 0) - return 0; + if (try_mtd() == 0) { + ret = 0; + goto internal_init_exit; + }
/* Initialize PCI access for flash enables */ - if (pci_init_common() != 0) - return 1; + if (pci_init_common() != 0) { + ret = 1; + goto internal_init_exit; + }
if (processor_flash_enable()) { msg_perr("Processor detection/init failed.\n" "Aborting.\n"); - return 1; + ret = 1; + goto internal_init_exit; }
#if IS_X86 @@ -238,8 +245,10 @@ msg_pwarn("Warning: The mainboard IDs set by -p internal:mainboard (%s:%s) do not\n" " match the current coreboot IDs of the mainboard (%s:%s).\n", board_vendor, board_model, cb_vendor, cb_model); - if (!force_boardmismatch) - return 1; + if (!force_boardmismatch) { + ret = 1; + goto internal_init_exit; + } msg_pinfo("Continuing anyway.\n"); } } @@ -281,8 +290,9 @@ if (ret == -2) { msg_perr("WARNING: No chipset found. Flash detection " "will most likely fail.\n"); - } else if (ret == ERROR_FATAL) - return ret; + } else if (ret == ERROR_FATAL) { + goto internal_init_exit; + }
#if IS_X86 /* Probe unconditionally for ITE Super I/O chips. This enables LPC->SPI translation on IT87* and @@ -291,7 +301,8 @@
if (board_flash_enable(board_vendor, board_model, cb_vendor, cb_model)) { msg_perr("Aborting to be safe.\n"); - return 1; + ret = 1; + goto internal_init_exit; } #endif
@@ -325,7 +336,13 @@ "========================================================================\n"); }
- return 0; + ret = 0; + +internal_init_exit: + free(board_vendor); + free(board_model); + + return ret; }
static void internal_chip_writeb(const struct flashctx *flash, uint8_t val, diff --git a/programmer.h b/programmer.h index 34ef33d..e42c6bb 100644 --- a/programmer.h +++ b/programmer.h @@ -272,7 +272,7 @@ #if CONFIG_INTERNAL == 1 /* board_enable.c */ int selfcheck_board_enables(void); -int board_parse_parameter(const char *boardstring, const char **vendor, const char **model); +int board_parse_parameter(const char *boardstring, char **vendor, char **model); void w836xx_ext_enter(uint16_t port); void w836xx_ext_leave(uint16_t port); void probe_superio_winbond(void);
-
David Hendricks (Code Review) -
Jacob Garber (Code Review)
-
Nico Huber (Code Review)