March 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: writeprotect_ranges.c: Check for underflow when using unsigned types
by Angel Pons (Code Review) 11 Mar '22

11 Mar '22

Attention is currently required from: Nico Huber, Light. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62733 ) Change subject: writeprotect_ranges.c: Check for underflow when using unsigned types ...................................................................... Patch Set 7: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62733/comment/9ea4ea98_9563a020 PS2, Line 12: Still the bug is not fixed :( > Done Ah, it's an underflow issue. -- To view, visit https://review.coreboot.org/c/flashrom/+/62733 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2766ef8e34f7121dad746e5f32a835d480ae1cad Gerrit-Change-Number: 62733 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Fri, 11 Mar 2022 09:56:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: test_build.sh: Allow WARNERROR to be overridden
by Angel Pons (Code Review) 11 Mar '22

11 Mar '22

Attention is currently required from: Nico Huber, Martin Roth. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62745 ) Change subject: test_build.sh: Allow WARNERROR to be overridden ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iea931e57f2a6992762566dc3dbaae8bb8df5b226 Gerrit-Change-Number: 62745 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Fri, 11 Mar 2022 09:51:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: writeprotect_ranges.c: Check for underflow when using unsigned types
by Nikolai Artemiev (Code Review) 11 Mar '22

11 Mar '22

Attention is currently required from: Nico Huber, Angel Pons, Light. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62733 ) Change subject: writeprotect_ranges.c: Check for underflow when using unsigned types ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62733 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2766ef8e34f7121dad746e5f32a835d480ae1cad Gerrit-Change-Number: 62733 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Fri, 11 Mar 2022 06:58:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
by Light (Code Review) 11 Mar '22

11 Mar '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62747 to look at the new patch set (#2). Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc ...................................................................... flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc In flashrom_image_write variables curcontents and oldcontents are dynamically allocated using malloc. These could remain uninitialized and when later used in need_erase could result in undefined behaviour. Similiar reasoning for variables hbuf, hdrs, tbuf in function prob_spi_sfdp. So allocate them using calloc to initialize them to zeroes or if allocating using malloc separately initialize them using a loop. Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c M sfdp.c M writeprotect_ranges.c 3 files changed, 5 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/62747/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab Gerrit-Change-Number: 62747 Gerrit-PatchSet: 2 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom.c: Initialize dynamically allocated memory using calloc
by Light (Code Review) 11 Mar '22

11 Mar '22

Light has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62747 ) Change subject: flashrom.c: Initialize dynamically allocated memory using calloc ...................................................................... flashrom.c: Initialize dynamically allocated memory using calloc In flashrom_image_write variables curcontents and oldcontents are dynamically allocated using malloc. These could remain uninitialized and when later used in need_erase could result in undefined behaviour. So allocate them using calloc to initialize them to zeroes or if allocating using malloc separately initialize them using a loop. Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M flashrom.c M writeprotect_ranges.c 2 files changed, 2 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/62747/1 diff --git a/flashrom.c b/flashrom.c index ac61259..f1fe651 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2036,10 +2036,10 @@ uint8_t *const newcontents = buffer; const uint8_t *const refcontents = refbuffer; - uint8_t *const curcontents = malloc(flash_size); + uint8_t *const curcontents = calloc(1, flash_size); uint8_t *oldcontents = NULL; if (verify_all) - oldcontents = malloc(flash_size); + oldcontents = calloc(1, flash_size); if (!curcontents || (verify_all && !oldcontents)) { msg_gerr("Out of memory!\n"); goto _free_ret; diff --git a/writeprotect_ranges.c b/writeprotect_ranges.c index b389126..dacce32 100644 --- a/writeprotect_ranges.c +++ b/writeprotect_ranges.c @@ -14,7 +14,6 @@ * GNU General Public License for more details. */ -#include <assert.h> #include "writeprotect.h" #include "chipdrivers.h" @@ -27,8 +26,6 @@ size_t bp = 0; size_t bp_max = 0; - assert(bits->bp_bit_count > 1); - for (size_t i = 0; i < bits->bp_bit_count; i++) { bp |= bits->bp[i] << i; bp_max |= 1 << i; -- To view, visit https://review.coreboot.org/c/flashrom/+/62747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab Gerrit-Change-Number: 62747 Gerrit-PatchSet: 1 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: tests: Add run_probe_lifecycle and add dummyflasher probe test
by Thomas Heijligen (Code Review) 11 Mar '22

11 Mar '22

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59741 ) Change subject: tests: Add run_probe_lifecycle and add dummyflasher probe test ...................................................................... Patch Set 4: (1 comment) File tests/lifecycle.c: https://review.coreboot.org/c/flashrom/+/59741/comment/ebef19ce_9fffec95 PS4, Line 34: flashrom_flash_release(flashctx); /* cleanup */ Only a valid flashctx should be released. Even if the `flashrom_flash_release` function has a protection against freeing a NULL pointer. (IMO) Or aborts `assert_int_equal` the test? -- To view, visit https://review.coreboot.org/c/flashrom/+/59741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9eb7fe3a436fbba5e70db957139fd26e00efec36 Gerrit-Change-Number: 59741 Gerrit-PatchSet: 4 Gerrit-Owner: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 11 Mar 2022 03:13:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: writeprotect_ranges.c: Check for underflow when using unsigned types
by Light (Code Review) 11 Mar '22

11 Mar '22

Attention is currently required from: Nico Huber, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Nico Huber, Nikolai Artemiev, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62733 to look at the new patch set (#7). Change subject: writeprotect_ranges.c: Check for underflow when using unsigned types ...................................................................... writeprotect_ranges.c: Check for underflow when using unsigned types size_t is an unsigned type so bp_max-2 causes an underflow. This results in undefined behaviour. So assert before that bits->bp_bit-count > 1 so that bp_max has a minimum value of 2. But the when bp_bit_count == 0 then bp == 0 and when bp_bit_count == 1 then bp == 0 or bp == 1 == bp_max. These cases are handled seaprately so the underflow will never occur. Ignore the tool for this issue. Change-Id: I2766ef8e34f7121dad746e5f32a835d480ae1cad Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M writeprotect_ranges.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/62733/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/62733 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2766ef8e34f7121dad746e5f32a835d480ae1cad Gerrit-Change-Number: 62733 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: writeprotect_ranges.c: Check for underflow when using unsigned types
by Light (Code Review) 11 Mar '22

11 Mar '22

Attention is currently required from: Nico Huber, Angel Pons, Nikolai Artemiev. Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62733 ) Change subject: writeprotect_ranges.c: Check for underflow when using unsigned types ...................................................................... Patch Set 6: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62733/comment/ff6a936c_5ebae696 PS2, Line 12: Still the bug is not fixed :( > The types can't help us because an underflow or a negative `bp_max - 2` both […] Done File tags: PS2: > Did you mean to add this file? Sorry, that was added by mistake. -- To view, visit https://review.coreboot.org/c/flashrom/+/62733 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2766ef8e34f7121dad746e5f32a835d480ae1cad Gerrit-Change-Number: 62733 Gerrit-PatchSet: 6 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 11 Mar 2022 03:01:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: serprog.c: Avoid calling memcpy with NULL pointer arguments
by Light (Code Review) 11 Mar '22

11 Mar '22

Attention is currently required from: Paul Menzel, Angel Pons. Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62726 ) Change subject: serprog.c: Avoid calling memcpy with NULL pointer arguments ...................................................................... Patch Set 7: (6 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62726/comment/1f78df71_9ef1b823 PS1, Line 7: Fixed > Fix Done https://review.coreboot.org/c/flashrom/+/62726/comment/81142cd9_9fb8e93e PS1, Line 7: n > Remove extra `n` at the end Done https://review.coreboot.org/c/flashrom/+/62726/comment/4d9def4f_f9e1fde2 PS1, Line 7: Fixed passed null value when expectingn nonnull > How about: […] Done https://review.coreboot.org/c/flashrom/+/62726/comment/fc73a52e_273e9342 PS1, Line 11: Just added a check before : memcpy for parms==NULL. > How about: […] Done File serprog.c: https://review.coreboot.org/c/flashrom/+/62726/comment/28883722_01f34ce2 PS1, Line 294: if > Please add a space after `if` Done https://review.coreboot.org/c/flashrom/+/62726/comment/eb10ccf1_ca6888e7 PS1, Line 295: memcpy(&(sp[1]), parms, parmlen); > We use tabs for indentation, and a tab is 8 characters wide. Please refer to https://flashrom. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/62726 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b Gerrit-Change-Number: 62726 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 11 Mar 2022 03:00:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pony_spi.c: Fix memory leak in function pony_init_spi
by Light (Code Review) 11 Mar '22

11 Mar '22

Attention is currently required from: Martin Roth, Paul Menzel, Angel Pons, Anastasia Klimchuk. Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62724 ) Change subject: pony_spi.c: Fix memory leak in function pony_init_spi ...................................................................... Patch Set 6: (8 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62724/comment/6504a275_85809e3e PS1, Line 7: Fixed > Imperative mood: Fix Done https://review.coreboot.org/c/flashrom/+/62724/comment/f927cd1e_6a48bd61 PS1, Line 9: Memory leaked was caused as data variable wasn't deallocated in some error cases where : the function returned without deallocatiing it. > Please wrap commit message lines at 72 characters, c.f. https://flashrom. […] Done https://review.coreboot.org/c/flashrom/+/62724/comment/58f18aec_1e0f7288 PS1, Line 11: > You can add information on how you tested this patch. […] Done Commit Message: https://review.coreboot.org/c/flashrom/+/62724/comment/848c65d1_0684d05f PS3, Line 11: ii > nit: just one `i` Done File pony_spi.c: https://review.coreboot.org/c/flashrom/+/62724/comment/16fc36ea_e5e1e26d PS1, Line 164: free(data); > We use tabs for indentation, and a tab is 8 characters wide. Please refer to https://flashrom. […] Done https://review.coreboot.org/c/flashrom/+/62724/comment/e63a67d6_9e1799e4 PS1, Line 182: free(data); > This isn't needed because the shutdown function has been registered, which will run `free(data)` alr […] Done https://review.coreboot.org/c/flashrom/+/62724/comment/176a6f26_043eec0b PS1, Line 248: free(data); > This isn't needed either, see my comment on line 182. Done https://review.coreboot.org/c/flashrom/+/62724/comment/65a2ac51_409a9c1b PS1, Line 255: free(data); > This won't work. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/62724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7910db94f63693e7f131836d4963e88cfdbec301 Gerrit-Change-Number: 62724 Gerrit-PatchSet: 6 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 11 Mar 2022 02:59:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022