September 2022 - flashrom-gerrit

[S] Change in flashrom[1.2.x]: pcidev.c: populate IDs with pci_fill_info()
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Daniel Verkamp, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Daniel Verkamp, Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67876 to review the following change. Change subject: pcidev.c: populate IDs with pci_fill_info() ...................................................................... pcidev.c: populate IDs with pci_fill_info() With pciutils 3.7.0, flashrom is unable to match any PCI devices by vendor/device ID because the vendor_id and device_id fields of struct pci_dev are not filled in. Call pci_fill_info() to request these identifiers before trying to match them against the supported device list. The pciutils ChangeLog for 3.7.0 mentions that the documentation and back-end behavior for pci_fill_info() was updated; it seems that a call to pci_fill_info() was always intended to be required, but some backends (such as the sysfs one used on Linux) would fill the identifier fields even when not requested by the user. The pci_fill_info() function and the PCI_FILL_IDENT flag have been available for all versions of pciutils since at least 2.0 from 1999, so it should be safe to add without any version checks. With this change, reading/writing a nicintel_spi boot ROM is successful. Signed-off-by: Daniel Verkamp <dverkamp(a)chromium.org> Change-Id: Ia011d4d801f8a54160e45a70b14b740e6dcc00ef Reviewed-on: https://review.coreboot.org/c/flashrom/+/46310 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M pcidev.c 1 file changed, 34 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/67876/1 diff --git a/pcidev.c b/pcidev.c index 54c1fd3..5057d8f 100644 --- a/pcidev.c +++ b/pcidev.c @@ -207,6 +207,7 @@ for (dev = pacc->devices; dev; dev = dev->next) { if (pci_filter_match(&filter, dev)) { + pci_fill_info(dev, PCI_FILL_IDENT); /* Check against list of supported devices. */ for (i = 0; devs[i].device_name != NULL; i++) if ((dev->vendor_id == devs[i].vendor_id) && -- To view, visit https://review.coreboot.org/c/flashrom/+/67876 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: Ia011d4d801f8a54160e45a70b14b740e6dcc00ef Gerrit-Change-Number: 67876 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: dmi: Correctly check for ERROR_PTR
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Angel Pons. Hello build bot (Jenkins), Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67875 to review the following change. Change subject: dmi: Correctly check for ERROR_PTR ...................................................................... dmi: Correctly check for ERROR_PTR For the physmap*() functions, NULL is considered valid return value. Fixes a segmentation fault when DMI tables can't be mapped. Tested on intel/eblake board with broken coreboot. Change-Id: Ic403c2940c2b91acbd113f0acfa3ce9ef6c6bb6c Signed-off-by: Nico Huber <nico.h(a)gmx.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/62611 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M dmi.c 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/67875/1 diff --git a/dmi.c b/dmi.c index c44221c..7488d61 100644 --- a/dmi.c +++ b/dmi.c @@ -164,7 +164,7 @@ unsigned int i = 0, j = 0; uint8_t *dmi_table_mem = physmap_ro("DMI Table", base, len); - if (dmi_table_mem == NULL) { + if (dmi_table_mem == ERROR_PTR) { msg_perr("Unable to access DMI Table\n"); return; } -- To view, visit https://review.coreboot.org/c/flashrom/+/67875 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: Ic403c2940c2b91acbd113f0acfa3ce9ef6c6bb6c Gerrit-Change-Number: 67875 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: linux_mtd: check ioctl() return value properly
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, Nikolai Artemiev, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67874 to review the following change. Change subject: linux_mtd: check ioctl() return value properly ...................................................................... linux_mtd: check ioctl() return value properly Make the linux_mtd driver treat any negative return value from the MEMERASE ioctl as an error. Previously it only treated -1 as an error. BUG=b:213561594,b:210973586,b:182223106 BRANCH=none TEST=builds Change-Id: I40cfbdee2ab608fbe6c17d9cac6ec53ff224d9a4 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/60996 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 linux_mtd.c 1 file changed, 26 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/67874/1 diff --git a/linux_mtd.c b/linux_mtd.c index 22702e9..316c8ed 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -280,9 +280,11 @@ .length = mtd_erasesize, }; - if (ioctl(fileno(dev_fp), MEMERASE, &erase_info) == -1) { - msg_perr("%s: ioctl: %s\n", __func__, strerror(errno)); - return 1; + int ret = ioctl(fileno(dev_fp), MEMERASE, &erase_info); + if (ret < 0) { + msg_perr("%s: MEMERASE ioctl call returned %d, error: %s\n", + __func__, ret, strerror(errno)); + return 1; } } -- To view, visit https://review.coreboot.org/c/flashrom/+/67874 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: I40cfbdee2ab608fbe6c17d9cac6ec53ff224d9a4 Gerrit-Change-Number: 67874 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: flashrom.8: add missing entry for `--flash-contents`
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Michael Niewöhner. Hello build bot (Jenkins), Michael Niewöhner, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67873 to review the following change. Change subject: flashrom.8: add missing entry for `--flash-contents` ...................................................................... flashrom.8: add missing entry for `--flash-contents` Change-Id: I64a8200a86329bd26a2069c5dc39430de9f8ba09 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/57807 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M flashrom.8.tmpl 1 file changed, 20 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/67873/1 diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index aa5bcd4..959e5fd 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -250,6 +250,13 @@ .B "\-\-flash\-size" Prints out the detected flash chips size. .TP +.B "\-\-flash\-contents <ref\-file>" +The file contents of +.BR <ref\-file> +will be used to decide which parts of the flash need to be written. Providing +this saves an initial read of the full flash chip. Be careful, if the provided +data doesn't actually match the flash contents, results are undefined. +.TP .B "\-L, \-\-list\-supported" List the flash chips, chipsets, mainboards, and external programmers (including PCI, USB, parallel port, and serial port based devices) -- To view, visit https://review.coreboot.org/c/flashrom/+/67873 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: I64a8200a86329bd26a2069c5dc39430de9f8ba09 Gerrit-Change-Number: 67873 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: jlink_spi: Reduce transfer size
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: zapb, Angel Pons. Hello zapb, build bot (Jenkins), Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67872 to review the following change. Change subject: jlink_spi: Reduce transfer size ...................................................................... jlink_spi: Reduce transfer size The maximum transfer size is too large for some devices and results in an USB timeout. Change-Id: If2c00b1524ec56740bdfe290096c3546cf375d73 Signed-off-by: Marc Schink <dev(a)zapb.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/48379 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Miklós Márton <martonmiklosqdev(a)gmail.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M jlink_spi.c 1 file changed, 18 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/67872/1 diff --git a/jlink_spi.c b/jlink_spi.c index 8a69b41..00e9c6d 100644 --- a/jlink_spi.c +++ b/jlink_spi.c @@ -34,7 +34,7 @@ * Maximum number of bytes that can be transferred at once via the JTAG * interface, see jaylink_jtag_io(). */ -#define JTAG_MAX_TRANSFER_SIZE (UINT16_MAX / 8) +#define JTAG_MAX_TRANSFER_SIZE (32768 / 8) /* * Default base frequency in Hz. Used when the base frequency can not be -- To view, visit https://review.coreboot.org/c/flashrom/+/67872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: If2c00b1524ec56740bdfe290096c3546cf375d73 Gerrit-Change-Number: 67872 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: zapb <dev(a)zapb.de> Gerrit-Attention: zapb <dev(a)zapb.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[1.2.x]: linux_mtd: Disable buffering on the mtd device
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Hung-Te Lin, Douglas Anderson, Angel Pons, Nikolai Artemiev, Patrick Rudolph. Hello Hung-Te Lin, build bot (Jenkins), Douglas Anderson, Angel Pons, Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67871 to review the following change. Change subject: linux_mtd: Disable buffering on the mtd device ...................................................................... linux_mtd: Disable buffering on the mtd device We open the device node for the MTD device with this: dev_fp = fopen(dev_path, "r+") In C fopen() is allowed to provide _buffered_ access to the file. That means that the standard library is allowed to read ahead and/or return cached data. That's really not what we want for something like this. Let's turn it off. This fixes a problem where flashrom would sometimes fail to "verify" that it erased the flash. The error message would look something like this: Erasing and writing flash chip... FAILED at 0x0000e220! Expected=0xff, Found=0xe9, failed byte count from 0x0000e200-0x0000e2ff: 0xdc failed byte count from 0x0000e000-0x0000efff: 0xffffffff ERASE_FAILED FAILED! Uh oh. Erase/write failed. Checking if anything changed. After the failure I could read the flash device with a new invocation of flashrom and I would see that, indeed, the erase had worked. Tracing in the kernel showed that when the failure happened we saw a pattern that looked like this: * Read 0x0b00 bytes starting at 0x0000d000 * Read 0x1000 bytes starting at 0x0000db00 * Erase 0x1000 bytes starting at 0x0000e000 ...and then there was _not_ a read after the erase. It can be assumed that, since userspace had already read 0xdb00 - 0xeaff that it was looking at old buffered data after the erase. Signed-off-by: Douglas Anderson <dianders(a)chromium.org> Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 Reviewed-on: https://review.coreboot.org/c/flashrom/+/50155 Reviewed-by: Hung-Te Lin <hungte(a)chromium.org> Reviewed-by: Patrick Rudolph <siro(a)das-labor.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M linux_mtd.c 1 file changed, 50 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/71/67871/1 diff --git a/linux_mtd.c b/linux_mtd.c index d2df95e..22702e9 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -340,6 +340,10 @@ msg_perr("Cannot open file stream for %s\n", dev_path); goto linux_mtd_setup_exit; } + ret = setvbuf(dev_fp, NULL, _IONBF, 0); + if (ret) + msg_pwarn("Failed to set MTD device to unbuffered: %d\n", ret); + msg_pinfo("Opened %s successfully\n", dev_path); ret = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/67871 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: I989afd83a33013b2756a0090d6b08245613215c6 Gerrit-Change-Number: 67871 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Douglas Anderson <dianders(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: helpers.c: Fix undefined behavior in strndup()
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67870 to review the following change. Change subject: helpers.c: Fix undefined behavior in strndup() ...................................................................... helpers.c: Fix undefined behavior in strndup() Using strlen() or strdup() inside strndup() is problematic: if the input string is not null-terminated, these functions can read past the end of the buffer, which triggers undefined behavior. Rewrite the function to never read past the provided `maxlen` bound. Change-Id: Id34127024085879228626fbad59af03268ec5255 Signed-off-by: Xiang Wang <merle(a)hardenedliux.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/49741 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M helpers.c 1 file changed, 28 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/70/67870/1 diff --git a/helpers.c b/helpers.c index c83cd2c..289848d 100644 --- a/helpers.c +++ b/helpers.c @@ -106,15 +106,16 @@ /* strndup is a POSIX function not present in MinGW */ char *strndup(const char *src, size_t maxlen) { - if (strlen(src) > maxlen) { - char *retbuf; - if ((retbuf = malloc(1 + maxlen)) != NULL) { - memcpy(retbuf, src, maxlen); - retbuf[maxlen] = '\0'; - } - return retbuf; + char *retbuf; + size_t len; + for (len = 0; len < maxlen; len++) + if (src[len] == '\0') + break; + if ((retbuf = malloc(1 + len)) != NULL) { + memcpy(retbuf, src, len); + retbuf[len] = '\0'; } - return strdup(src); + return retbuf; } #endif -- To view, visit https://review.coreboot.org/c/flashrom/+/67870 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: Id34127024085879228626fbad59af03268ec5255 Gerrit-Change-Number: 67870 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: dediprog: Fix segmentation fault on no device found
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Medicine Yeh, Stefan Reinauer, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Medicine Yeh, Stefan Reinauer, Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67869 to review the following change. Change subject: dediprog: Fix segmentation fault on no device found ...................................................................... dediprog: Fix segmentation fault on no device found libusb_exit() call is done by dediprog_open() under the ret == 1 condition. Removing this line has no impact on any flow and side effect of the program. Change-Id: I38b3f3ee3f9d46845df1404791f4a4782320aa7c Signed-off-by: Medicine Yeh <medicinehy(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/48688 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M dediprog.c 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/67869/1 diff --git a/dediprog.c b/dediprog.c index 175e099..99df7b0 100644 --- a/dediprog.c +++ b/dediprog.c @@ -1188,7 +1188,6 @@ ret = dediprog_open(i); if (ret == -1) { /* no dev */ - libusb_exit(usb_ctx); return 1; } else if (ret == -2) { /* busy dev */ -- To view, visit https://review.coreboot.org/c/flashrom/+/67869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: I38b3f3ee3f9d46845df1404791f4a4782320aa7c Gerrit-Change-Number: 67869 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Medicine Yeh <medicinehy(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Medicine Yeh <medicinehy(a)gmail.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: chipset_enable.c: Validate physmap() return rcrb value
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Sam McNally, Edward O'Callaghan, Angel Pons. Hello Sam McNally, build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67868 to review the following change. Change subject: chipset_enable.c: Validate physmap() return rcrb value ...................................................................... chipset_enable.c: Validate physmap() return rcrb value Validate the physical mapping in enable_flash_silvermont(). Change-Id: Icc5a799a06b3f310d9a191fa5eb99b255b20d79d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/48225 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sam McNally <sammc(a)google.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M chipset_enable.c 1 file changed, 18 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/68/67868/1 diff --git a/chipset_enable.c b/chipset_enable.c index 5195b95..bbd0465 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -988,6 +988,8 @@ /* Handle GCS (in RCRB) */ void *rcrb = physmap("BYT RCRB", rcba, 4); + if (rcrb == ERROR_PTR) + return ERROR_FATAL; const enum chipbustype boot_buses = enable_flash_ich_report_gcs(dev, ich_generation, rcrb); physunmap(rcrb, 4); -- To view, visit https://review.coreboot.org/c/flashrom/+/67868 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: Icc5a799a06b3f310d9a191fa5eb99b255b20d79d Gerrit-Change-Number: 67868 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: chipset_enable.c: check return value from rphysmap() call
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Sam McNally, Edward O'Callaghan, Angel Pons. Hello Sam McNally, build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67867 to review the following change. Change subject: chipset_enable.c: check return value from rphysmap() call ...................................................................... chipset_enable.c: check return value from rphysmap() call Port from the ChromiumOS fork of flashrom. Change-Id: I8075fe5f80ac0da5280d2f0de6829ed3a2496476 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/46444 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sam McNally <sammc(a)google.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M chipset_enable.c 1 file changed, 18 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/67/67867/1 diff --git a/chipset_enable.c b/chipset_enable.c index 0dfe267..5195b95 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1002,6 +1002,8 @@ uint32_t sbase = pci_read_long(dev, 0x54) & 0xfffffe00; msg_pdbg("SPI_BASE_ADDRESS = 0x%x\n", sbase); void *spibar = rphysmap("BYT SBASE", sbase, 512); /* Last defined address on Bay Trail is 0x100 */ + if (spibar == ERROR_PTR) + return ERROR_FATAL; /* Enable Flash Writes. * Silvermont-based: BCR at SBASE + 0xFC (some bits of BCR are also accessible via BC at IBASE + 0x1C). -- To view, visit https://review.coreboot.org/c/flashrom/+/67867 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.2.x Gerrit-Change-Id: I8075fe5f80ac0da5280d2f0de6829ed3a2496476 Gerrit-Change-Number: 67867 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022