September 2022 - flashrom-gerrit

[S] Change in flashrom[master]: cli_classic.c: Early init of layout obscures invalid memory access
by Nikolai Artemiev (Code Review) 20 Sep '22

20 Sep '22

Attention is currently required from: Edward O'Callaghan. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67723 ) Change subject: cli_classic.c: Early init of layout obscures invalid memory access ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/67723 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3df61b873da27f6945d60916a1c3713dedfc3924 Gerrit-Change-Number: 67723 Gerrit-PatchSet: 2 Gerrit-Owner: 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-Comment-Date: Tue, 20 Sep 2022 05:49:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: layout.c: Validate _layout_entry_by_name() arguments before use
by Nikolai Artemiev (Code Review) 20 Sep '22

20 Sep '22

Attention is currently required from: Edward O'Callaghan. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67722 ) Change subject: layout.c: Validate _layout_entry_by_name() arguments before use ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/67722 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a19c0e586f8575b8b3c2c02b5afad312efacfc9 Gerrit-Change-Number: 67722 Gerrit-PatchSet: 2 Gerrit-Owner: 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-Comment-Date: Tue, 20 Sep 2022 05:48:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: cli_classic.c: Add error messages for invalid --wp-region use
by Nikolai Artemiev (Code Review) 20 Sep '22

20 Sep '22

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/67725 ) Change subject: cli_classic.c: Add error messages for invalid --wp-region use ...................................................................... cli_classic.c: Add error messages for invalid --wp-region use Print warning if --wp-region is used without a layout file or the layout file doesn't contain the region. BUG=b:247055486 TEST=builds Change-Id: Ie606ba7f8a423405099679ca62169c395d994b5d Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M cli_classic.c 1 file changed, 27 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/67725/1 diff --git a/cli_classic.c b/cli_classic.c index e384845..dd8d5e2 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -1113,9 +1113,18 @@ if (any_wp_op) { if (set_wp_region && wp_region) { - ret = flashrom_layout_get_region_range(layout, wp_region, &wp_start, &wp_len); - if (ret) + + if (!layout) { + msg_gerr("Error: A flash layout must be specified to use --wp-region.\n"); + ret = 1; goto out_release; + } + + ret = flashrom_layout_get_region_range(layout, wp_region, &wp_start, &wp_len); + if (ret) { + msg_gerr("Error: Region %s not found in flash layout.\n", wp_region); + goto out_release; + } set_wp_range = true; } ret = wp_cli( -- To view, visit https://review.coreboot.org/c/flashrom/+/67725 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie606ba7f8a423405099679ca62169c395d994b5d Gerrit-Change-Number: 67725 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: cli_classic.c: Early init of layout obscures invalid memory access
by Edward O'Callaghan (Code Review) 20 Sep '22

20 Sep '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/67723 ) Change subject: cli_classic.c: Early init of layout obscures invalid memory access ...................................................................... cli_classic.c: Early init of layout obscures invalid memory access BUG=b:247055486 TEST=builds Change-Id: I3df61b873da27f6945d60916a1c3713dedfc3924 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M cli_classic.c 1 file changed, 17 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/67723/1 diff --git a/cli_classic.c b/cli_classic.c index bac31d1..e384845 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -538,7 +538,7 @@ bool dont_verify_it = false, dont_verify_all = false; bool list_supported = false; bool show_progress = false; - struct flashrom_layout *layout = NULL; + struct flashrom_layout *layout; static const struct programmer_entry *prog = NULL; enum { OPTION_IFD = 0x0100, @@ -1105,6 +1105,9 @@ flashrom_flash_getsize(fill_flash)) || process_include_args(layout, include_args))) { ret = 1; goto out_shutdown; + } else { + layout = NULL; + msg_gdbg("Valid layout could not be found without image.\n"); } flashrom_layout_set(fill_flash, layout); -- To view, visit https://review.coreboot.org/c/flashrom/+/67723 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3df61b873da27f6945d60916a1c3713dedfc3924 Gerrit-Change-Number: 67723 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: layout.c: Validate _layout_entry_by_name() arguments before use
by Edward O'Callaghan (Code Review) 20 Sep '22

20 Sep '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/67722 ) Change subject: layout.c: Validate _layout_entry_by_name() arguments before use ...................................................................... layout.c: Validate _layout_entry_by_name() arguments before use It maybe the case that a layout could not be derived which would result in layout logic feed with a NULL pointer. Validate this case and be defensive to validate the name argument as well. BUG=b:247055486 TEST=builds Change-Id: I2a19c0e586f8575b8b3c2c02b5afad312efacfc9 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M layout.c 1 file changed, 19 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/67722/1 diff --git a/layout.c b/layout.c index 5916d66..0212699 100644 --- a/layout.c +++ b/layout.c @@ -58,6 +58,8 @@ const struct flashrom_layout *const layout, const char *name) { struct romentry *entry = NULL; + if (!layout || !name) + return NULL; while ((entry = mutable_layout_next(layout, entry))) { if (!strcmp(entry->name, name)) return entry; -- To view, visit https://review.coreboot.org/c/flashrom/+/67722 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a19c0e586f8575b8b3c2c02b5afad312efacfc9 Gerrit-Change-Number: 67722 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: tests/parade_lspcon.c: Replace spaces with tabs
by Anastasia Klimchuk (Code Review) 20 Sep '22

20 Sep '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67643 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tests/parade_lspcon.c: Replace spaces with tabs ...................................................................... tests/parade_lspcon.c: Replace spaces with tabs Spaces were accidentally introduced in previous commit, replace with tabs as it should be. Change-Id: Ic3a7d3004b8358953917a6666bcf8f1cdade02fd Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67643 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M tests/parade_lspcon.c 1 file changed, 27 insertions(+), 10 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/tests/parade_lspcon.c b/tests/parade_lspcon.c index a1c9f72..9d7bd23 100644 --- a/tests/parade_lspcon.c +++ b/tests/parade_lspcon.c @@ -19,19 +19,19 @@ /* Same macros as in parade_lspcon.c programmer. */ /* FIXME(aklm): should driver register maps be defined in `include/drivers/` for sharing with tests? */ -#define REGISTER_ADDRESS 0x4a -#define SPISTATUS 0x9e -#define SPISTATUS_SECTOR_ERASE_FINISHED 0 -#define SWSPICTL 0x93 -#define SWSPICTL_ENABLE_READBACK 0x8 -#define SWSPI_RDATA 0x91 +#define REGISTER_ADDRESS 0x4a +#define SPISTATUS 0x9e +#define SPISTATUS_SECTOR_ERASE_FINISHED 0 +#define SWSPICTL 0x93 +#define SWSPICTL_ENABLE_READBACK 0x8 +#define SWSPI_RDATA 0x91 /* Macros for test run. */ -#define DATA_TO_READ 0 -#define MAX_REG_BUF_LEN 2 +#define DATA_TO_READ 0 +#define MAX_REG_BUF_LEN 2 struct parade_lspcon_io_state { - unsigned long addr; /* Address to read and write */ - uint8_t reg_buf[MAX_REG_BUF_LEN]; /* Last value written to the register address */ + unsigned long addr; /* Address to read and write */ + uint8_t reg_buf[MAX_REG_BUF_LEN]; /* Last value written to the register address */ }; static int parade_lspcon_ioctl(void *state, int fd, unsigned long request, va_list args) -- To view, visit https://review.coreboot.org/c/flashrom/+/67643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic3a7d3004b8358953917a6666bcf8f1cdade02fd Gerrit-Change-Number: 67643 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(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: 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[master]: stlinkv3_spi: fix uninitialized pointer
by Anastasia Klimchuk (Code Review) 20 Sep '22

20 Sep '22

Attention is currently required from: Thomas Heijligen, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67700 ) Change subject: stlinkv3_spi: fix uninitialized pointer ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: > There is a patch covering the same topic: CB:66225 Thomas what do you think is the state of CB:66225 ? It does seem to cover a bit more (3 lines vs 1 line), but is it ready or waiting for action from patch owner? I can see two possible ways: either CB:66225 gets finished and merged and then this one not needed, or this one gets merged and then CB:66225 gets rebased and the work continues. -- To view, visit https://review.coreboot.org/c/flashrom/+/67700 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibaf25f67186724d9045ade849026782c3eac4952 Gerrit-Change-Number: 67700 Gerrit-PatchSet: 1 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Tue, 20 Sep 2022 01:14:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: writeprotect: Add some debug logging if wp_verify fails
by Evan Benn (Code Review) 20 Sep '22

20 Sep '22

Attention is currently required from: Nikolai Artemiev. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67720 ) Change subject: writeprotect: Add some debug logging if wp_verify fails ...................................................................... Patch Set 1: (1 comment) File writeprotect.c: https://review.coreboot.org/c/flashrom/+/67720/comment/57fd4423_2531ff28 PS1, Line 166: return FLASHROM_WP_ERR_READ_FAILED; This does slightly change the behaviour where before an early failure would be a VERIFY_FAILED but now could become a READ_FAILED. Probably nothing, worth a thought though. -- To view, visit https://review.coreboot.org/c/flashrom/+/67720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5fcaf767570418f90ae44826a1135d9b49653033 Gerrit-Change-Number: 67720 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 20 Sep 2022 01:00:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: Add write protect bits to W25Q64JW...M
by Evan Benn (Code Review) 20 Sep '22

20 Sep '22

Attention is currently required from: Nikolai Artemiev. Hello Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67719 to look at the new patch set (#2). Change subject: flashchips: Add write protect bits to W25Q64JW...M ...................................................................... flashchips: Add write protect bits to W25Q64JW...M https://www.winbond.com/hq/support/documentation/levelOne.jsp?__locale=en&D… BUG=b:245996788 BRANCH=None TEST=None Change-Id: Idf2289b7c90724ececc122d2a05c7cae3af2cf62 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M flashchips.c 1 file changed, 28 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/19/67719/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67719 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idf2289b7c90724ececc122d2a05c7cae3af2cf62 Gerrit-Change-Number: 67719 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: writeprotect: Add some debug logging if wp_verify fails
by Evan Benn (Code Review) 20 Sep '22

20 Sep '22

Attention is currently required from: Nikolai Artemiev. Hello Nikolai Artemiev, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/67720 to review the following change. Change subject: writeprotect: Add some debug logging if wp_verify fails ...................................................................... writeprotect: Add some debug logging if wp_verify fails Change-Id: I5fcaf767570418f90ae44826a1135d9b49653033 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M writeprotect.c 1 file changed, 18 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/67720/1 diff --git a/writeprotect.c b/writeprotect.c index 57d0f9b..c32bee1 100644 --- a/writeprotect.c +++ b/writeprotect.c @@ -155,6 +155,7 @@ return FLASHROM_WP_ERR_WRITE_FAILED; } + enum flashrom_wp_result ret = FLASHROM_WP_OK; /* Verify each register */ for (enum flash_reg reg = STATUS1; reg < MAX_REGISTERS; reg++) { if (!write_masks[reg]) @@ -164,14 +165,18 @@ if (wp_read_register(flash, reg, &value)) return FLASHROM_WP_ERR_READ_FAILED; + msg_cdbg2("wp_verify reg:%u value:0x%x\n", reg, value); uint8_t actual = value & write_masks[reg]; uint8_t expected = reg_values[reg] & write_masks[reg]; - if (actual != expected) - return FLASHROM_WP_ERR_VERIFY_FAILED; + if (actual != expected) { + msg_cdbg("wp_verify failed: reg:%u actual:0x%x expected:0x%x\n", reg, + actual, expected); + ret = FLASHROM_WP_ERR_VERIFY_FAILED; + } } - return FLASHROM_WP_OK; + return ret; } /** Get the range selected by a WP configuration. */ -- To view, visit https://review.coreboot.org/c/flashrom/+/67720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5fcaf767570418f90ae44826a1135d9b49653033 Gerrit-Change-Number: 67720 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.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