- flashrom-gerrit - mail.coreboot.org

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by DigitalDJ (Code Review) 06 Sep '24

06 Sep '24

Attention is currently required from: Aarya, Anastasia Klimchuk. DigitalDJ has posted comments on this change by DigitalDJ. ( https://review.coreboot.org/c/flashrom/+/84234?usp=email ) Change subject: erasure_layout: Fix init_eraseblock segmentation fault ...................................................................... Patch Set 5: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/84234/comment/3266f689_7f59b533?us… : PS3, Line 9: Fix a segmentation fault that is caused by accessing an invalid "subedata" pointer on the last iteration of the init_eraseblock loop. Instead, short circuit the loop condition to check the sub block index first, and do not access the invalid pointer if it is the last sub block. > Could you please wrap commit message to 72 chars, as in our dev guide, thanks! […] Done https://review.coreboot.org/c/flashrom/+/84234/comment/7959c895_cf62b4ac?us… : PS3, Line 10: > The one thing I want to ask you: […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/84234?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Gerrit-Change-Number: 84234 Gerrit-PatchSet: 5 Gerrit-Owner: DigitalDJ Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: DigitalDJ Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 06 Sep 2024 15:33:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by DigitalDJ (Code Review) 06 Sep '24

06 Sep '24

Attention is currently required from: Aarya, DigitalDJ. Hello Aarya, Anastasia Klimchuk, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84234?usp=email to look at the new patch set (#5). Change subject: erasure_layout: Fix init_eraseblock segmentation fault ...................................................................... erasure_layout: Fix init_eraseblock segmentation fault Fix a segmentation fault that is caused by accessing an invalid "subedata" pointer on the last iteration of the init_eraseblock loop. Instead, short circuit the loop condition to check the sub block index first, and do not access the invalid pointer if it is the last sub block. Issue was encountered in: - OS: OpenBSD 7.5 amd64 - Compiler: clang 16.0.6 - Chip: Macronix MX25U6435E/F BUG=https://ticket.coreboot.org/issues/555 Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Signed-off-by: Grant Pannell <grant(a)digitaldj.net> --- M erasure_layout.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84234/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/84234?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Gerrit-Change-Number: 84234 Gerrit-PatchSet: 5 Gerrit-Owner: DigitalDJ Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: DigitalDJ

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by DigitalDJ (Code Review) 06 Sep '24

06 Sep '24

Attention is currently required from: Aarya, DigitalDJ. Hello Aarya, Anastasia Klimchuk, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84234?usp=email to look at the new patch set (#4). Change subject: erasure_layout: Fix init_eraseblock segmentation fault ...................................................................... erasure_layout: Fix init_eraseblock segmentation fault Fix a segmentation fault that is caused by accessing an invalid "subedata" pointer on the last iteration of the init_eraseblock loop. Instead, short circuit the loop condition to check the sub block index first, and do not access the invalid pointer if it is the last sub block. BUG=https://ticket.coreboot.org/issues/555 Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Signed-off-by: Grant Pannell <grant(a)digitaldj.net> --- M erasure_layout.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84234/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/84234?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Gerrit-Change-Number: 84234 Gerrit-PatchSet: 4 Gerrit-Owner: DigitalDJ Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: DigitalDJ

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by Anastasia Klimchuk (Code Review) 06 Sep '24

06 Sep '24

Attention is currently required from: Aarya, DigitalDJ. Anastasia Klimchuk has posted comments on this change by DigitalDJ. ( https://review.coreboot.org/c/flashrom/+/84234?usp=email ) Change subject: erasure_layout: Fix init_eraseblock segmentation fault ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/84234/comment/350182da_6b6b268e?us… : PS3, Line 9: Fix a segmentation fault that is caused by accessing an invalid "subedata" pointer on the last iteration of the init_eraseblock loop. Instead, short circuit the loop condition to check the sub block index first, and do not access the invalid pointer if it is the last sub block. Could you please wrap commit message to 72 chars, as in our dev guide, thanks! https://flashrom.org/dev_guide/development_guide.html#commit-message-1 -- To view, visit https://review.coreboot.org/c/flashrom/+/84234?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Gerrit-Change-Number: 84234 Gerrit-PatchSet: 3 Gerrit-Owner: DigitalDJ Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: DigitalDJ Gerrit-Comment-Date: Fri, 06 Sep 2024 13:56:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by Anastasia Klimchuk (Code Review) 06 Sep '24

06 Sep '24

Attention is currently required from: Aarya, DigitalDJ. Anastasia Klimchuk has posted comments on this change by DigitalDJ. ( https://review.coreboot.org/c/flashrom/+/84234?usp=email ) Change subject: erasure_layout: Fix init_eraseblock segmentation fault ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: Thank you for the patch. Great work! Commit Message: https://review.coreboot.org/c/flashrom/+/84234/comment/878bb722_8b81ad70?us… : PS3, Line 10: The one thing I want to ask you: If you can add to commit message the details about your environment: OS and its version, and probably compiler? Because my theory is that it is not segfaulting for everyone everywhere, it's useful to have exact environment described in the patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/84234?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Gerrit-Change-Number: 84234 Gerrit-PatchSet: 3 Gerrit-Owner: DigitalDJ Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: DigitalDJ Gerrit-Comment-Date: Fri, 06 Sep 2024 13:54:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by DigitalDJ (Code Review) 06 Sep '24

06 Sep '24

Attention is currently required from: Aarya. Hello Aarya, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84234?usp=email to look at the new patch set (#3). Change subject: erasure_layout: Fix init_eraseblock segmentation fault ...................................................................... erasure_layout: Fix init_eraseblock segmentation fault Fix a segmentation fault that is caused by accessing an invalid "subedata" pointer on the last iteration of the init_eraseblock loop. Instead, short circuit the loop condition to check the sub block index first, and do not access the invalid pointer if it is the last sub block. BUG=https://ticket.coreboot.org/issues/555 Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Signed-off-by: Grant Pannell <grant(a)digitaldj.net> --- M erasure_layout.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84234/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/84234?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Gerrit-Change-Number: 84234 Gerrit-PatchSet: 3 Gerrit-Owner: DigitalDJ Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by DigitalDJ (Code Review) 06 Sep '24

06 Sep '24

Attention is currently required from: Aarya. Hello Aarya, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84234?usp=email to look at the new patch set (#2). Change subject: erasure_layout: Fix init_eraseblock segmentation fault ...................................................................... erasure_layout: Fix init_eraseblock segmentation fault Fix a segmentation fault that is caused by accessing an invalid "subedata" pointer on the last iteration of the init_eraseblock loop. Instead, short circuit the condition and do not access the invalid pointer if it is the last sub block. BUG=https://ticket.coreboot.org/issues/555 Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Signed-off-by: Grant Pannell <grant(a)digitaldj.net> --- M erasure_layout.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84234/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84234?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Gerrit-Change-Number: 84234 Gerrit-PatchSet: 2 Gerrit-Owner: DigitalDJ Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by DigitalDJ (Code Review) 06 Sep '24

06 Sep '24

DigitalDJ has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84234?usp=email ) Change subject: erasure_layout: Fix init_eraseblock segmentation fault ...................................................................... erasure_layout: Fix init_eraseblock segmentation fault Fix a segmentation fault that is caused by accessing an invalid "subedata" pointer on the last iteration of the init_eraseblock loop. Instead, short circuit the condition and do not access the invalid pointer if it is the last sub block. BUG=https://ticket.coreboot.org/issues/555 Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Signed-off-by: Grant Pannell <grant(a)digitaldj.net> --- M erasure_layout.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84234/1 diff --git a/erasure_layout.c b/erasure_layout.c index a7eaa2d..0a64030 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -52,8 +52,8 @@ edata->first_sub_block_index = *sub_block_index; struct eraseblock_data *subedata = &layout[idx - 1].layout_list[*sub_block_index]; - while (subedata->start_addr >= start_addr && subedata->end_addr <= end_addr && - *sub_block_index < layout[idx-1].block_count) { + while (*sub_block_index < layout[idx-1].block_count && + subedata->start_addr >= start_addr && subedata->end_addr <= end_addr) { (*sub_block_index)++; subedata++; } -- To view, visit https://review.coreboot.org/c/flashrom/+/84234?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I61bf0d93aa9f0b2b420b146be16fcd5124f0dc5d Gerrit-Change-Number: 84234 Gerrit-PatchSet: 1 Gerrit-Owner: DigitalDJ

1 0

[M] Change in flashrom[main]: Fix FEATURE_NO_ERASE chips and add test for them
by Peter Marheine (Code Review) 06 Sep '24

06 Sep '24

Attention is currently required from: Anastasia Klimchuk, Miklós Márton. Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84203?usp=email ) Change subject: Fix FEATURE_NO_ERASE chips and add test for them ...................................................................... Patch Set 3: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/84203/comment/a73930a0_5943095a?us… : PS3, Line 487: for (int i = 0; opcode[i]; i++) { This will always crash if `spi_get_opcode_from_erasefn` doesn't know the eraser, which should be probably be fixed to gracefully bail out. `FEATURE_NO_ERASE` seems to more correctly indicate that the chip doesn't need to be erased before writing, so I don't think it's correct to check that flag when seeing which erase options are supported: it's still meaningful to want to erase a chip without writing it, and some chips might have more efficient erase commands than rewriting with "erased" values which we'd want to support (and probe for support). Since `spi_get_opcode_from_erasefn` might not know the opcode for a custom erasefn (including `BLOCK_ERASE_EMULATION`) and chips that don't require erase on write might still have usable erase opcodes, I think we should just skip this loop if `opcode == NULL`. Possibly also a comment should be added to `FEATURE_NO_ERASE` to document that it controls erase before write specifically. -- To view, visit https://review.coreboot.org/c/flashrom/+/84203?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I582fe00da0715e9b5e92fcc9d15d5a90a2615117 Gerrit-Change-Number: 84203 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 06 Sep 2024 03:22:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: ichspi: Add RaptorPoint PCH support
by Sergii Dmytruk (Code Review) 05 Sep '24

05 Sep '24

Attention is currently required from: Krystian Hebel, Maciej Pijanowski, Michał Kopeć, Michał Żygowski. Sergii Dmytruk has posted comments on this change by Maciej Pijanowski. ( https://review.coreboot.org/c/flashrom/+/83854?usp=email ) Change subject: ichspi: Add RaptorPoint PCH support ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/83854?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I13ac52d5400c0e2260e12d605077fc2182c379ef Gerrit-Change-Number: 83854 Gerrit-PatchSet: 8 Gerrit-Owner: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Filip Gołaś <filip.golas(a)3mdeb.com> Gerrit-CC: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Thu, 05 Sep 2024 16:22:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit