September 2024 - flashrom-gerrit

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by Anastasia Klimchuk (Code Review) Sept. 7, 2024

Sept. 7, 2024

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 5: Code-Review+2 (1 comment) Patchset: PS5: Just for future: you don't need to vote on your own patch, we assume the author agrees with the own patch (otherwise why would you send it? :) ). In the dev guidelines we require the patch to be approved by at least one person who is not the author. Exception is when patch is co-developed by multiple authors, then they all approve that work. You don't have to change anything here, leave your vote as is, it's not an issue. That's just info for future! -- 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: DigitalDJ Gerrit-Comment-Date: Sat, 07 Sep 2024 08:44:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix init_eraseblock segmentation fault
by DigitalDJ (Code Review) Sept. 6, 2024

Sept. 6, 2024

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: (1 comment) Patchset: PS3: > Thank you for the patch. […] Thank you for taking a look and validating the findings! -- 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:46:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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) Sept. 6, 2024

Sept. 6, 2024

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) Sept. 6, 2024

Sept. 6, 2024

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) Sept. 6, 2024

Sept. 6, 2024

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) Sept. 6, 2024

Sept. 6, 2024

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) Sept. 6, 2024

Sept. 6, 2024

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) Sept. 6, 2024

Sept. 6, 2024

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) Sept. 6, 2024

Sept. 6, 2024

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) Sept. 6, 2024

Sept. 6, 2024

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