September 2022 - flashrom-gerrit

[S] Change in flashrom[1.2.x]: it87spi.c: Prevent use-after-free bug
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/+/67866 to review the following change. Change subject: it87spi.c: Prevent use-after-free bug ...................................................................... it87spi.c: Prevent use-after-free bug The memory for the `param` string is aliased by `dualbiosindex_suffix`. 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: I79f18f6077c77c0cbb8bfa431e17f9b079f11c95 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/46551 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M it87spi.c 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/66/67866/1 diff --git a/it87spi.c b/it87spi.c index f5955fe..7b38492 100644 --- a/it87spi.c +++ b/it87spi.c @@ -134,12 +134,13 @@ char *dualbiosindex_suffix; errno = 0; long chip_index = strtol(param, &dualbiosindex_suffix, 0); - free(param); if (errno != 0 || *dualbiosindex_suffix != '\0' || chip_index < 0 || chip_index > 1) { msg_perr("DualBIOS: Invalid chip index requested - choose 0 or 1.\n"); + free(param); exit_conf_mode_ite(port); return 1; } + free(param); if (chip_index != (tmp & 1)) { msg_pdbg("DualBIOS: Previous chip index: %d\n", tmp & 1); sio_write(port, 0xEF, (tmp & 0xFE) | chip_index); -- To view, visit https://review.coreboot.org/c/flashrom/+/67866 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: I79f18f6077c77c0cbb8bfa431e17f9b079f11c95 Gerrit-Change-Number: 67866 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]: spi25: Debug flashrom crash when Write Protect is ON
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Stefan Reinauer, Edward O'Callaghan, SANTHOSH JANARDHANA HASSAN. Hello build bot (Jenkins), Stefan Reinauer, Edward O'Callaghan, SANTHOSH JANARDHANA HASSAN, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67865 to review the following change. Change subject: spi25: Debug flashrom crash when Write Protect is ON ...................................................................... spi25: Debug flashrom crash when Write Protect is ON When hardware write protect is applied, flashrom crashed and generate coredump. spi_disable_blockprotect_generic() calls flash->chip->printlock() method when disable was failed, but this method is optional, can be NULL depends on type of flashrom chip. NULL pointer check before call is added to avoid crash. BRANCH=none BUG=b:129083894 TEST=Run on Mistral P2 (On CR50 console, run "wp disable") flashrom --wp-range 0 0x400000 flashrom --wp-enable (On CR50 console, run "wp enable") flashrom -r /tmp/test.bin Verify "Block protection could not be disabled!" is shown, but flash read completes. Signed-off-by: Yuji Sasaki <sasakiy(a)chromium.org> Change-Id: I81094ab5f16a85871fc9869a2e285eddbbbdec4e Reviewed-on: https://chromium-review.googlesource.com/1535140 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator(a)appspot.gserviceaccount.com> Tested-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Stefan Reinauer <reinauer(a)google.com> Reviewed-by: SANTHOSH JANARDHANA HASSAN <sahassan(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/40468 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M spi25_statusreg.c 1 file changed, 38 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/67865/1 diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 8cd5a28..bb09e58 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -168,7 +168,8 @@ status = spi_read_status_register(flash); if ((status & bp_mask) != 0) { msg_cerr("Block protection could not be disabled!\n"); - flash->chip->printlock(flash); + if (flash->chip->printlock) + flash->chip->printlock(flash); return 1; } msg_cdbg("disabled.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/67865 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: I81094ab5f16a85871fc9869a2e285eddbbbdec4e Gerrit-Change-Number: 67865 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: SANTHOSH JANARDHANA HASSAN <sahassan(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Yuji Sasaki <sasakiy(a)chromium.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: SANTHOSH JANARDHANA HASSAN <sahassan(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: chipset_enable.c: Disable SPI on ICH7 if booted from LPC
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Angel Pons, Elyes Haouas. Hello build bot (Jenkins), Angel Pons, Elyes Haouas, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67863 to review the following change. Change subject: chipset_enable.c: Disable SPI on ICH7 if booted from LPC ...................................................................... chipset_enable.c: Disable SPI on ICH7 if booted from LPC Commit 92d6a86 ("Refactor Intel Chipset Enables") eliminated a check to disable SPI when ICH7 has booted from LPC, as the hardware does not support it. Therefore, when flashrom probes the SPI bus, it times out waiting for the hardware to react, for each and every SPI flash chip. This results in very long delays and countless instances of the error: Error: SCIP never cleared! To prevent this, bring back part of the lost check. Probing for LPC and FWH when booted from SPI does not seem to cause any problems on desktop mainboards with ICH7, so don't disable LPC nor FWH if that is the case. Tested on ECS 945G-M4 (ICH7, boots from LPC), works without errors. Change-Id: I5e59e66a2dd16b07f2dca410997fce38ab9c8fd1 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/40401 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: HAOUAS Elyes <ehaouas(a)noos.fr> --- M chipset_enable.c 1 file changed, 35 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/67863/1 diff --git a/chipset_enable.c b/chipset_enable.c index 84e4b6b..0dfe267 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -757,6 +757,14 @@ if (ret_fwh == ERROR_FATAL) return ret_fwh; + /* + * It seems that the ICH7 does not support SPI and LPC chips at the same time. When booted + * from LPC, the SCIP bit will never clear, which causes long delays and many error messages. + * To avoid this, we will not enable SPI on ICH7 when the southbridge is strapped to LPC. + */ + if (ich_generation == CHIPSET_ICH7 && (boot_buses & BUS_LPC)) + return 0; + /* SPIBAR is at RCRB+0x3020 for ICH[78], Tunnel Creek and Centerton, and RCRB+0x3800 for ICH9. */ uint16_t spibar_offset; switch (ich_generation) { -- To view, visit https://review.coreboot.org/c/flashrom/+/67863 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: I5e59e66a2dd16b07f2dca410997fce38ab9c8fd1 Gerrit-Change-Number: 67863 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: stlinkv3_spi: Avoid division by zero
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Patrick Georgi, Stefan Reinauer, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Patrick Georgi, Stefan Reinauer, Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67864 to review the following change. Change subject: stlinkv3_spi: Avoid division by zero ...................................................................... stlinkv3_spi: Avoid division by zero Change-Id: I08c0612f3fea59add9bde2fb3cc5c4b5c3756516 Found-by: Coverity Scan #1412744 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/40653 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M stlinkv3_spi.c 1 file changed, 19 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/67864/1 diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 7338911..d2988b6 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -491,10 +491,11 @@ speed_str = extract_programmer_param("spispeed"); if (speed_str) { sck_freq_kHz = strtoul(speed_str, &endptr, 0); - if (*endptr) { + if (*endptr || sck_freq_kHz == 0) { msg_perr("The spispeed parameter passed with invalid format: %s\n", speed_str); - msg_perr("Please pass the parameter with a simple number in kHz\n"); + msg_perr("Please pass the parameter " + "with a simple non-zero number in kHz\n"); return -1; } free(speed_str); -- To view, visit https://review.coreboot.org/c/flashrom/+/67864 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: I08c0612f3fea59add9bde2fb3cc5c4b5c3756516 Gerrit-Change-Number: 67864 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: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> 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]: libflashrom.c: Use casts on enum conversions
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Angel Pons, Elyes Haouas. Hello build bot (Jenkins), Angel Pons, Elyes Haouas, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67861 to review the following change. Change subject: libflashrom.c: Use casts on enum conversions ...................................................................... libflashrom.c: Use casts on enum conversions This allows flashrom to build with GCC 10. Change-Id: I2166cdf3681452631ef8e980face2924e9a6c81a Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/41775 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: HAOUAS Elyes <ehaouas(a)noos.fr> --- M libflashrom.c 1 file changed, 19 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/67861/1 diff --git a/libflashrom.c b/libflashrom.c index 0dec22e..51ad7d9 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -188,7 +188,8 @@ for (; i < boards_known_size; ++i) { supported_boards[i].vendor = binfo[i].vendor; supported_boards[i].name = binfo[i].name; - supported_boards[i].working = binfo[i].working; + supported_boards[i].working = + (enum flashrom_test_state) binfo[i].working; } } else { msg_gerr("Memory allocation error!\n"); @@ -226,7 +227,8 @@ supported_chipsets[i].chipset = chipset[i].device_name; supported_chipsets[i].vendor_id = chipset[i].vendor_id; supported_chipsets[i].chipset_id = chipset[i].device_id; - supported_chipsets[i].status = chipset[i].status; + supported_chipsets[i].status = + (enum flashrom_test_state) chipset[i].status; } } else { msg_gerr("Memory allocation error!\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/67861 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: I2166cdf3681452631ef8e980face2924e9a6c81a Gerrit-Change-Number: 67861 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: spi95: Check for success before using send_command's returned data
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Patrick Georgi, Angel Pons. Hello build bot (Jenkins), Patrick Georgi, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67862 to review the following change. Change subject: spi95: Check for success before using send_command's returned data ...................................................................... spi95: Check for success before using send_command's returned data If the transfer failed, the data might be invalid. Change-Id: I3ad9daa00a54e2a3954983cec91b6685f1a98880 Found-By: Coverity Scan #1405870 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/40970 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M spi95.c 1 file changed, 20 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/67862/1 diff --git a/spi95.c b/spi95.c index ecb2c1d..976f99a 100644 --- a/spi95.c +++ b/spi95.c @@ -33,12 +33,15 @@ static const unsigned char cmd[ST_M95_RDID_OUTSIZE_MAX] = { ST_M95_RDID }; unsigned char readarr[ST_M95_RDID_INSIZE]; uint32_t id1, id2; + int ret; uint32_t rdid_outsize = ST_M95_RDID_2BA_OUTSIZE; // 16 bit address if (flash->chip->total_size * KiB > 64 * KiB) rdid_outsize = ST_M95_RDID_3BA_OUTSIZE; // 24 bit address - spi_send_command(flash, rdid_outsize, sizeof(readarr), cmd, readarr); + ret = spi_send_command(flash, rdid_outsize, sizeof(readarr), cmd, readarr); + if (ret) + return ret; id1 = readarr[0]; // manufacture id id2 = (readarr[1] << 8) | readarr[2]; // SPI family code + model id -- To view, visit https://review.coreboot.org/c/flashrom/+/67862 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: I3ad9daa00a54e2a3954983cec91b6685f1a98880 Gerrit-Change-Number: 67862 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.2.x]: test_build.sh: Move build test procedure to repository
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Patrick Georgi, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Patrick Georgi, Edward O'Callaghan, Angel Pons, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67860 to review the following change. Change subject: test_build.sh: Move build test procedure to repository ...................................................................... test_build.sh: Move build test procedure to repository Instead of hard coding the test procedure on qa.coreboot.org, allow running a script in the repo instead. The server is already adapted to do that, so once there's a test_build.sh file in the toplevel directory, it's run in place of the default operation. The content of this change mirrors the default operation exactly so should serve as a good starting point. The script is executed in an encapsulate[0] context with the workspace, /tmp and $HOME/.ccache writable, everything else read-only and network disabled. It should return 0 on success, anything else on failure, as is normal for UNIX processes. [0] https://review.coreboot.org/cgit/encapsulate.git (Backported minus the Meson support) Change-Id: I37a8e925d1b283c3b8f87cb3d0f1ed8920f2cf95 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/46894 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/62617 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: Nico Huber <nico.h(a)gmx.de> --- A test_build.sh 1 file changed, 39 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/67860/1 diff --git a/test_build.sh b/test_build.sh new file mode 100755 index 0000000..3ab5319 --- /dev/null +++ b/test_build.sh @@ -0,0 +1,4 @@ +#!/bin/sh +set -e + +make CONFIG_EVERYTHING=yes WARNERROR=yes -- To view, visit https://review.coreboot.org/c/flashrom/+/67860 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: I37a8e925d1b283c3b8f87cb3d0f1ed8920f2cf95 Gerrit-Change-Number: 67860 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: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Georgi <patrick(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.1.x]: pcidev: Always fetch ident info
by Nico Huber (Code Review) 26 Sep '22

26 Sep '22

Attention is currently required from: Thomas Heijligen, Daniel Verkamp. Hello build bot (Jenkins), Daniel Verkamp, Thomas Heijligen, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67859 to review the following change. Change subject: pcidev: Always fetch ident info ...................................................................... pcidev: Always fetch ident info As discovered earlier[1], the `vendor_id` and `device_id` fields are not always automatically set. However, we use these fields throughout flash- rom. To not lose track when we actually fetched them, let's always call pci_fill_info(PCI_FILL_IDENT) before returning a `pci_dev` handle. [1] Commit ca2e3bce0 (pcidev.c: populate IDs with pci_fill_info()) Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Ticket: https://ticket.coreboot.org/issues/367 Reviewed-on: https://review.coreboot.org/c/flashrom/+/64573 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Daniel Verkamp <dverkamp(a)chromium.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> --- M board_enable.c M internal.c 2 files changed, 39 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/67859/1 diff --git a/board_enable.c b/board_enable.c index 00809f7..aec69fc 100644 --- a/board_enable.c +++ b/board_enable.c @@ -1510,6 +1510,7 @@ /* First, look for a known LPC bridge */ for (dev = pacc->devices; dev; dev = dev->next) { + pci_fill_info(dev, PCI_FILL_IDENT); uint16_t device_class; /* libpci before version 2.2.4 does not store class info. */ device_class = pci_read_word(dev, PCI_CLASS_DEVICE); diff --git a/internal.c b/internal.c index 1d6cff6..b596c53 100644 --- a/internal.c +++ b/internal.c @@ -25,9 +25,12 @@ { struct pci_dev *temp; - for (temp = pacc->devices; temp; temp = temp->next) - if (pci_filter_match(&filter, temp)) + for (temp = pacc->devices; temp; temp = temp->next) { + if (pci_filter_match(&filter, temp)) { + pci_fill_info(temp, PCI_FILL_IDENT); return temp; + } + } return NULL; } @@ -45,8 +48,10 @@ if (pci_filter_match(&filter, temp)) { /* Read PCI class */ tmp2 = pci_read_word(temp, 0x0a); - if (tmp2 == devclass) + if (tmp2 == devclass) { + pci_fill_info(temp, PCI_FILL_IDENT); return temp; + } } return NULL; @@ -61,9 +66,12 @@ filter.vendor = vendor; filter.device = device; - for (temp = pacc->devices; temp; temp = temp->next) - if (pci_filter_match(&filter, temp)) + for (temp = pacc->devices; temp; temp = temp->next) { + if (pci_filter_match(&filter, temp)) { + pci_fill_info(temp, PCI_FILL_IDENT); return temp; + } + } return NULL; } @@ -83,8 +91,10 @@ if ((card_vendor == pci_read_word(temp, PCI_SUBSYSTEM_VENDOR_ID)) && (card_device == - pci_read_word(temp, PCI_SUBSYSTEM_ID))) + pci_read_word(temp, PCI_SUBSYSTEM_ID))) { + pci_fill_info(temp, PCI_FILL_IDENT); return temp; + } } return NULL; -- To view, visit https://review.coreboot.org/c/flashrom/+/67859 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.1.x Gerrit-Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Gerrit-Change-Number: 67859 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Daniel Verkamp <dverkamp(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.1.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/+/67858 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/58/67858/1 diff --git a/pcidev.c b/pcidev.c index f5ad819..b0fb487 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/+/67858 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.1.x Gerrit-Change-Id: Ia011d4d801f8a54160e45a70b14b740e6dcc00ef Gerrit-Change-Number: 67858 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.1.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/+/67857 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/57/67857/1 diff --git a/linux_mtd.c b/linux_mtd.c index 75dce1f..7052e5d 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/+/67857 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.1.x Gerrit-Change-Id: I40cfbdee2ab608fbe6c17d9cac6ec53ff224d9a4 Gerrit-Change-Number: 67857 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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022