May 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: dummyflasher.c: Fix typo in log message
by Edward O'Callaghan (Code Review) May 28, 2021

May 28, 2021

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54910 ) Change subject: dummyflasher.c: Fix typo in log message ...................................................................... dummyflasher.c: Fix typo in log message multipler ---> multiple Change-Id: I16fbc0072c82ac152af5f1d65df196cdd113e03b Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54910 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M dummyflasher.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, but someone else must approve diff --git a/dummyflasher.c b/dummyflasher.c index 560dbdc..dcf97b5 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -831,7 +831,7 @@ if (tmp) { size = strtol(tmp, NULL, 10); if (size <= 0 || (size % 1024 != 0)) { - msg_perr("%s: Chip size is not a multipler of 1024: %s\n", + msg_perr("%s: Chip size is not a multiple of 1024: %s\n", __func__, tmp); free(tmp); return 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/54910 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I16fbc0072c82ac152af5f1d65df196cdd113e03b Gerrit-Change-Number: 54910 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: dummyflasher.c: Prevent use-after-free bug
by Edward O'Callaghan (Code Review) May 28, 2021

May 28, 2021

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54909 ) Change subject: dummyflasher.c: Prevent use-after-free bug ...................................................................... dummyflasher.c: Prevent use-after-free bug The memory for the `status` string is aliased by the `endptr` pointer. Moreover, `errno` could have been modified by the call to `free()`. Therefore, only free the former when there are no more uses of either. Change-Id: I1b56834004fe18918213a7df0a09a8a7ecb56985 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54909 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M dummyflasher.c 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, but someone else must approve diff --git a/dummyflasher.c b/dummyflasher.c index 5defec0..560dbdc 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -962,12 +962,13 @@ if (status) { errno = 0; data->emu_status = strtoul(status, &endptr, 0); - free(status); if (errno != 0 || status == endptr) { + free(status); msg_perr("Error: initial status register specified, " "but the value could not be converted.\n"); return 1; } + free(status); msg_pdbg("Initial status register is set to 0x%02x.\n", data->emu_status); } -- To view, visit https://review.coreboot.org/c/flashrom/+/54909 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b56834004fe18918213a7df0a09a8a7ecb56985 Gerrit-Change-Number: 54909 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: dummyflasher.c: Move `flashchip_contents` allocation
by Edward O'Callaghan (Code Review) May 28, 2021

May 28, 2021

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54908 ) Change subject: dummyflasher.c: Move `flashchip_contents` allocation ...................................................................... dummyflasher.c: Move `flashchip_contents` allocation Place `flashchip_contents` allocation next to the code that initialises the newly-allocated buffer. This also avoids leaking it if the handling of `spi_status` fails. Change-Id: Ie907ac88dfe4ca018c97d9ce6ce042b4ffacf36a Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54908 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M dummyflasher.c 1 file changed, 6 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, but someone else must approve diff --git a/dummyflasher.c b/dummyflasher.c index 717f8d1..5defec0 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -957,12 +957,6 @@ } free(tmp); - data->flashchip_contents = malloc(data->emu_chip_size); - if (!data->flashchip_contents) { - msg_perr("Out of memory!\n"); - return 1; - } - #ifdef EMULATE_SPI_CHIP status = extract_programmer_param("spi_status"); if (status) { @@ -979,6 +973,12 @@ } #endif + data->flashchip_contents = malloc(data->emu_chip_size); + if (!data->flashchip_contents) { + msg_perr("Out of memory!\n"); + return 1; + } + msg_pdbg("Filling fake flash chip with 0x%02x, size %i\n", data->erase_to_zero ? 0x00 : 0xff, data->emu_chip_size); memset(data->flashchip_contents, data->erase_to_zero ? 0x00 : 0xff, data->emu_chip_size); -- To view, visit https://review.coreboot.org/c/flashrom/+/54908 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie907ac88dfe4ca018c97d9ce6ce042b4ffacf36a Gerrit-Change-Number: 54908 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: dummyflasher.c: Don't leak `emu_persistent_image`
by Edward O'Callaghan (Code Review) May 28, 2021

May 28, 2021

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54905 ) Change subject: dummyflasher.c: Don't leak `emu_persistent_image` ...................................................................... dummyflasher.c: Don't leak `emu_persistent_image` Ensure `emu_persistent_image` doesn't end up leaking memory. Change-Id: I76529973cefcc6a1472681e1f4da8239fcbf07a6 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54905 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M dummyflasher.c 1 file changed, 3 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/dummyflasher.c b/dummyflasher.c index 325263d..717f8d1 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -634,9 +634,8 @@ write_buf_to_file(emu_data->flashchip_contents, emu_data->emu_chip_size, emu_data->emu_persistent_image); - free(emu_data->emu_persistent_image); - emu_data->emu_persistent_image = NULL; } + free(emu_data->emu_persistent_image); free(emu_data->flashchip_contents); } #endif @@ -1001,6 +1000,7 @@ if (read_buf_from_file(data->flashchip_contents, data->emu_chip_size, data->emu_persistent_image)) { msg_perr("Unable to read %s\n", data->emu_persistent_image); + free(data->emu_persistent_image); free(data->flashchip_contents); return 1; } @@ -1012,6 +1012,7 @@ dummy_init_out: if (register_shutdown(dummy_shutdown, data)) { + free(data->emu_persistent_image); free(data->flashchip_contents); free(data); return 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/54905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I76529973cefcc6a1472681e1f4da8239fcbf07a6 Gerrit-Change-Number: 54905 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: dummyflasher.c: Fix use-after-free issue
by Edward O'Callaghan (Code Review) May 28, 2021

May 28, 2021

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/55014 ) Change subject: dummyflasher.c: Fix use-after-free issue ...................................................................... dummyflasher.c: Fix use-after-free issue Issue accidentally introduced in commit e3707bbf4. BUG=none BRANCH=none TEST=builds Change-Id: Ie12d6474ef5a7bb1afbbb9ec50b31b577da1a81b Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/55014 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M dummyflasher.c 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, but someone else must approve diff --git a/dummyflasher.c b/dummyflasher.c index b7cfab8..325263d 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -700,12 +700,13 @@ tmp = extract_programmer_param("spi_write_256_chunksize"); if (tmp) { data->spi_write_256_chunksize = strtoul(tmp, &endptr, 0); - free(tmp); if (*endptr != '\0' || data->spi_write_256_chunksize < 1) { msg_perr("invalid spi_write_256_chunksize\n"); + free(tmp); return 1; } } + free(tmp); tmp = extract_programmer_param("spi_blacklist"); if (tmp) { -- To view, visit https://review.coreboot.org/c/flashrom/+/55014 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie12d6474ef5a7bb1afbbb9ec50b31b577da1a81b Gerrit-Change-Number: 55014 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: serprog.c: Use braces in both branches of conditional statement
by Edward O'Callaghan (Code Review) May 28, 2021

May 28, 2021

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54862 ) Change subject: serprog.c: Use braces in both branches of conditional statement ...................................................................... serprog.c: Use braces in both branches of conditional statement As per the coding style, if one branch of a conditional statement needs braces, all other branches need to have braces as well. BUG=b:185191942 TEST=builds Change-Id: I4fdccbd66f0351af98811bf7c2d8e15f0a99d852 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54862 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M serprog.c 1 file changed, 13 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/serprog.c b/serprog.c index a70a289..e8c0d75 100644 --- a/serprog.c +++ b/serprog.c @@ -564,8 +564,9 @@ if (baud_str == NULL || *baud_str == '\0') { baud = -1; msg_pdbg("No baudrate specified, using the hardware's defaults.\n"); - } else + } else { baud = atoi(baud_str); // FIXME: replace atoi with strtoul + } if (strlen(device) > 0) { sp_fd = sp_openserport(device, baud); if (sp_fd == SER_INV_FD) { @@ -727,11 +728,11 @@ goto init_err_cleanup_exit; } if (strlen(f_spi_suffix) == 1) { - if (!strcasecmp(f_spi_suffix, "M")) + if (!strcasecmp(f_spi_suffix, "M")) { f_spi_req *= 1000000; - else if (!strcasecmp(f_spi_suffix, "k")) + } else if (!strcasecmp(f_spi_suffix, "k")) { f_spi_req *= 1000; - else { + } else { msg_perr("Error: Garbage following 'spispeed' value.\n"); free(spispeed); goto init_err_cleanup_exit; @@ -747,17 +748,18 @@ buf[2] = (f_spi_req >> (2 * 8)) & 0xFF; buf[3] = (f_spi_req >> (3 * 8)) & 0xFF; - if (sp_check_commandavail(S_CMD_S_SPI_FREQ) == 0) + if (sp_check_commandavail(S_CMD_S_SPI_FREQ) == 0) { msg_pwarn(MSGHEADER "Warning: Setting the SPI clock rate is not supported!\n"); - else if (sp_docommand(S_CMD_S_SPI_FREQ, 4, buf, 4, buf) == 0) { + } else if (sp_docommand(S_CMD_S_SPI_FREQ, 4, buf, 4, buf) == 0) { f_spi = buf[0]; f_spi |= buf[1] << (1 * 8); f_spi |= buf[2] << (2 * 8); f_spi |= buf[3] << (3 * 8); msg_pdbg(MSGHEADER "Requested to set SPI clock frequency to %u Hz. " "It was actually set to %u Hz\n", f_spi_req, f_spi); - } else + } else { msg_pwarn(MSGHEADER "Setting SPI clock rate to %u Hz failed!\n", f_spi_req); + } } free(spispeed); bt = serprog_buses_supported; @@ -871,10 +873,12 @@ if (sp_docommand(S_CMD_S_PIN_STATE, 1, &en, 0, NULL) != 0) { msg_perr("Error: could not enable output buffers\n"); goto init_err_cleanup_exit; - } else + } else { msg_pdbg(MSGHEADER "Output drivers enabled\n"); - } else + } + } else { msg_pdbg(MSGHEADER "Warning: Programmer does not support toggling its output drivers\n"); + } sp_prev_was_write = 0; sp_streamed_transmit_ops = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/54862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4fdccbd66f0351af98811bf7c2d8e15f0a99d852 Gerrit-Change-Number: 54862 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: serprog.c: Separate shutdown from failed init cleanup
by Edward O'Callaghan (Code Review) May 28, 2021

May 28, 2021

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/54861 ) Change subject: serprog.c: Separate shutdown from failed init cleanup ...................................................................... serprog.c: Separate shutdown from failed init cleanup Shutdown function was covering two different jobs here: 1) the actual shutdown which is run at the end of the driver's lifecycle and 2) cleanup in cases when initialisation failed. Now, shutdown is only doing its main job (#1), and the driver itself is doing cleanup when init fails (#2). The good thing is that now resources are released/closed immediately in cases when init fails (vs shutdown function which was run at some point later), and the driver leaves clean space after itself if init fails. And very importantly this unlocks API change which plans to move register_shutdown inside register master API, see https://review.coreboot.org/c/flashrom/+/51761 BUG=b:185191942 TEST=builds Change-Id: Idf4ed62c19667e18cc807913180c48cb8c978805 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/54861 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M serprog.c 1 file changed, 27 insertions(+), 22 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/serprog.c b/serprog.c index e6028aa..a70a289 100644 --- a/serprog.c +++ b/serprog.c @@ -635,33 +635,30 @@ return 1; } - if (register_shutdown(serprog_shutdown, NULL)) - return 1; - msg_pdbg(MSGHEADER "connected - attempting to synchronize\n"); sp_check_avail_automatic = 0; if (sp_synchronize()) - return 1; + goto init_err_cleanup_exit; msg_pdbg(MSGHEADER "Synchronized\n"); if (sp_docommand(S_CMD_Q_IFACE, 0, NULL, 2, &iface)) { msg_perr("Error: NAK to query interface version\n"); - return 1; + goto init_err_cleanup_exit; } if (iface != 1) { msg_perr("Error: Unknown interface version: %d\n", iface); - return 1; + goto init_err_cleanup_exit; } msg_pdbg(MSGHEADER "Interface version ok.\n"); if (sp_docommand(S_CMD_Q_CMDMAP, 0, NULL, 32, sp_cmdmap)) { msg_perr("Error: query command map not supported\n"); - return 1; + goto init_err_cleanup_exit; } sp_check_avail_automatic = 1; @@ -688,10 +685,10 @@ if (sp_check_commandavail(S_CMD_O_SPIOP) == 0) { msg_perr("Error: SPI operation not supported while the " "bustype is SPI\n"); - return 1; + goto init_err_cleanup_exit; } if (sp_docommand(S_CMD_S_BUSTYPE, 1, &bt, 0, NULL)) - return 1; + goto init_err_cleanup_exit; /* Success of any of these commands is optional. We don't need the programmer to tell us its limits, but if it doesn't, we will assume stuff, so it's in the programmers best interest @@ -727,7 +724,7 @@ if (errno != 0 || spispeed == f_spi_suffix) { msg_perr("Error: Could not convert 'spispeed'.\n"); free(spispeed); - return 1; + goto init_err_cleanup_exit; } if (strlen(f_spi_suffix) == 1) { if (!strcasecmp(f_spi_suffix, "M")) @@ -737,12 +734,12 @@ else { msg_perr("Error: Garbage following 'spispeed' value.\n"); free(spispeed); - return 1; + goto init_err_cleanup_exit; } } else if (strlen(f_spi_suffix) > 1) { msg_perr("Error: Garbage following 'spispeed' value.\n"); free(spispeed); - return 1; + goto init_err_cleanup_exit; } buf[0] = (f_spi_req >> (0 * 8)) & 0xFF; @@ -765,38 +762,38 @@ free(spispeed); bt = serprog_buses_supported; if (sp_docommand(S_CMD_S_BUSTYPE, 1, &bt, 0, NULL)) - return 1; + goto init_err_cleanup_exit; } if (serprog_buses_supported & BUS_NONSPI) { if (sp_check_commandavail(S_CMD_O_INIT) == 0) { msg_perr("Error: Initialize operation buffer " "not supported\n"); - return 1; + goto init_err_cleanup_exit; } if (sp_check_commandavail(S_CMD_O_DELAY) == 0) { msg_perr("Error: Write to opbuf: " "delay not supported\n"); - return 1; + goto init_err_cleanup_exit; } /* S_CMD_O_EXEC availability checked later. */ if (sp_check_commandavail(S_CMD_R_BYTE) == 0) { msg_perr("Error: Single byte read not supported\n"); - return 1; + goto init_err_cleanup_exit; } /* This could be translated to single byte reads (if missing), * but now we don't support that. */ if (sp_check_commandavail(S_CMD_R_NBYTES) == 0) { msg_perr("Error: Read n bytes not supported\n"); - return 1; + goto init_err_cleanup_exit; } if (sp_check_commandavail(S_CMD_O_WRITEB) == 0) { msg_perr("Error: Write to opbuf: " "write byte not supported\n"); - return 1; + goto init_err_cleanup_exit; } if (sp_docommand(S_CMD_Q_WRNMAXLEN, 0, NULL, 3, rbuf)) { @@ -815,7 +812,7 @@ if (!sp_write_n_buf) { msg_perr("Error: cannot allocate memory for " "Write-n buffer\n"); - return 1; + goto init_err_cleanup_exit; } sp_write_n_bytes = 0; } @@ -853,12 +850,12 @@ if (sp_check_commandavail(S_CMD_O_EXEC) == 0) { msg_perr("Error: Execute operation buffer not " "supported\n"); - return 1; + goto init_err_cleanup_exit; } if (sp_docommand(S_CMD_O_INIT, 0, NULL, 0, NULL)) { msg_perr("Error: NAK to initialize operation buffer\n"); - return 1; + goto init_err_cleanup_exit; } if (sp_docommand(S_CMD_Q_OPBUF, 0, NULL, 2, @@ -873,20 +870,28 @@ uint8_t en = 1; if (sp_docommand(S_CMD_S_PIN_STATE, 1, &en, 0, NULL) != 0) { msg_perr("Error: could not enable output buffers\n"); - return 1; + goto init_err_cleanup_exit; } else msg_pdbg(MSGHEADER "Output drivers enabled\n"); } else msg_pdbg(MSGHEADER "Warning: Programmer does not support toggling its output drivers\n"); + sp_prev_was_write = 0; sp_streamed_transmit_ops = 0; sp_streamed_transmit_bytes = 0; sp_opbuf_usage = 0; + + if (register_shutdown(serprog_shutdown, NULL)) + goto init_err_cleanup_exit; if (serprog_buses_supported & BUS_SPI) register_spi_master(&spi_master_serprog, NULL); if (serprog_buses_supported & BUS_NONSPI) register_par_master(&par_master_serprog, serprog_buses_supported & BUS_NONSPI, NULL); return 0; + +init_err_cleanup_exit: + serprog_shutdown(NULL); + return 1; } void serprog_delay(unsigned int usecs) -- To view, visit https://review.coreboot.org/c/flashrom/+/54861 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idf4ed62c19667e18cc807913180c48cb8c978805 Gerrit-Change-Number: 54861 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: dummyflasher.c: Fix use-after-free issue
by Anastasia Klimchuk (Code Review) May 27, 2021

May 27, 2021

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55014 ) Change subject: dummyflasher.c: Fix use-after-free issue ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/55014 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie12d6474ef5a7bb1afbbb9ec50b31b577da1a81b Gerrit-Change-Number: 55014 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 27 May 2021 22:37:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher.c: Fix use-after-free issue
by Angel Pons (Code Review) May 27, 2021

May 27, 2021

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55014 ) Change subject: dummyflasher.c: Fix use-after-free issue ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: Thanks. -- To view, visit https://review.coreboot.org/c/flashrom/+/55014 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie12d6474ef5a7bb1afbbb9ec50b31b577da1a81b Gerrit-Change-Number: 55014 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 27 May 2021 12:48:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher.c: Replace atoi() with strtoul()
by Edward O'Callaghan (Code Review) May 27, 2021

May 27, 2021

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54854 ) Change subject: dummyflasher.c: Replace atoi() with strtoul() ...................................................................... Patch Set 3: (1 comment) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/54854/comment/7e36d786_cc759264 PS3, Line 715: free(tmp); > Sigh, this introduces a use-after-free bug... CB:46551 and CB:54909 fixed the same kind of thing. […] damm it. thanks Angle fixed here https://review.coreboot.org/c/flashrom/+/55014 I do want to not get too much in the way of Anastasia project though, I just wanted to unblock a couple of things. Thanks again so much. -- To view, visit https://review.coreboot.org/c/flashrom/+/54854 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib9e66016a2f4ce2d13b833c261f900cab80916b7 Gerrit-Change-Number: 54854 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 27 May 2021 12:41:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0