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

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

28 May '21

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) 28 May '21

28 May '21

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) 28 May '21

28 May '21

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) 28 May '21

28 May '21

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) 27 May '21

27 May '21

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) 27 May '21

27 May '21

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) 27 May '21

27 May '21

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

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

27 May '21

Edward O'Callaghan has uploaded this change for review. ( 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> --- M dummyflasher.c 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/14/55014/1 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: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: dummyflasher.c: Replace atoi() with strtoul()
by Angel Pons (Code Review) 27 May '21

27 May '21

Attention is currently required from: Edward O'Callaghan. Angel Pons 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/a848741b_d7567fd3 PS3, Line 715: free(tmp); Sigh, this introduces a use-after-free bug... CB:46551 and CB:54909 fixed the same kind of thing. This strtoul() pattern repeats itself a lot. Why isn't it a helper function? -- 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 27 May 2021 10:42:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ogp_spi.c: Refactor singleton states into reentrant pattern
by Angel Pons (Code Review) 27 May '21

27 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54996 ) Change subject: ogp_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File ogp_spi.c: https://review.coreboot.org/c/flashrom/+/54996/comment/627d843b_a6381caa PS1, Line 158: if nit: missing space after `if` -- To view, visit https://review.coreboot.org/c/flashrom/+/54996 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6b8f6ecd7c9949ffb4098bf56a8936393283382f Gerrit-Change-Number: 54996 Gerrit-PatchSet: 1 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 27 May 2021 10:29:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021