September 2022 - flashrom-gerrit

[S] Change in flashrom[1.2.x]: stlinkv3_spi: Avoid division by zero
by Felix Singer (Code Review) 29 Sep '22

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67864 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67864 Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M stlinkv3_spi.c 1 file changed, 21 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved 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: 2 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: Felix Singer <felixsinger(a)posteo.net> 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-MessageType: merged

1 0

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

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67863 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67863 Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M chipset_enable.c 1 file changed, 38 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Elyes Haouas: Looks good to me, approved Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved 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: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

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

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67862 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67862 Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M spi95.c 1 file changed, 22 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved 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: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.2.x]: libflashrom.c: Use casts on enum conversions
by Felix Singer (Code Review) 29 Sep '22

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67861 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67861 Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr> --- M libflashrom.c 1 file changed, 22 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Elyes Haouas: Looks good to me, approved Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved 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: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

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

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67860 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67860 Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- A test_build.sh 1 file changed, 41 insertions(+), 0 deletions(-) Approvals: Nico Huber: Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved 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: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[1.0.x]: pcidev: Always fetch ident info
by Felix Singer (Code Review) 29 Sep '22

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67845 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )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()) Backported to older versions where pcidev handling was much more scattered. 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 Reviewed-on: https://review.coreboot.org/c/flashrom/+/67845 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M board_enable.c M internal.c 2 files changed, 44 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/board_enable.c b/board_enable.c index 3b6755e..97bb15a 100644 --- a/board_enable.c +++ b/board_enable.c @@ -1514,6 +1514,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 cb02753..7628988 100644 --- a/internal.c +++ b/internal.c @@ -30,9 +30,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; } @@ -50,8 +53,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; @@ -66,9 +71,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; } @@ -88,8 +96,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/+/67845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: Iae2511178bec44343cbe902722fdca9eda036059 Gerrit-Change-Number: 67845 Gerrit-PatchSet: 3 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.0.x]: pcidev.c: populate IDs with pci_fill_info()
by Felix Singer (Code Review) 29 Sep '22

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67844 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67844 Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M pcidev.c 1 file changed, 36 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/pcidev.c b/pcidev.c index f4e5542..4bc32c1 100644 --- a/pcidev.c +++ b/pcidev.c @@ -211,6 +211,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/+/67844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: Ia011d4d801f8a54160e45a70b14b740e6dcc00ef Gerrit-Change-Number: 67844 Gerrit-PatchSet: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.0.x]: chipset_enable.c: Validate physmap() return rcrb value
by Felix Singer (Code Review) 29 Sep '22

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67843 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67843 Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M chipset_enable.c 1 file changed, 21 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/chipset_enable.c b/chipset_enable.c index 67ce761..6220416 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -920,6 +920,8 @@ /* Handle GCS (in RCRB) */ void *rcrb = physmap("BYT RCRB", rcba, 4); + if (rcrb == ERROR_PTR) + return ERROR_FATAL; enable_flash_ich_report_gcs(dev, ich_generation, rcrb); physunmap(rcrb, 4); -- To view, visit https://review.coreboot.org/c/flashrom/+/67843 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: Icc5a799a06b3f310d9a191fa5eb99b255b20d79d Gerrit-Change-Number: 67843 Gerrit-PatchSet: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.0.x]: chipset_enable.c: check return value from rphysmap() call
by Felix Singer (Code Review) 29 Sep '22

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67842 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67842 Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M chipset_enable.c 1 file changed, 21 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/chipset_enable.c b/chipset_enable.c index 3437792..67ce761 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -934,6 +934,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/+/67842 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I8075fe5f80ac0da5280d2f0de6829ed3a2496476 Gerrit-Change-Number: 67842 Gerrit-PatchSet: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.0.x]: it87spi.c: Prevent use-after-free bug
by Felix Singer (Code Review) 29 Sep '22

29 Sep '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67841 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67841 Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M it87spi.c 1 file changed, 22 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/it87spi.c b/it87spi.c index 0a1e894..a2188fc 100644 --- a/it87spi.c +++ b/it87spi.c @@ -139,12 +139,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/+/67841 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.0.x Gerrit-Change-Id: I79f18f6077c77c0cbb8bfa431e17f9b079f11c95 Gerrit-Change-Number: 67841 Gerrit-PatchSet: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit September 2022