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
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
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
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
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
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
Attention is currently required from: Martin Roth, Light.
Hello build bot (Jenkins), Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62724
to look at the new patch set (#6).
Change subject: pony_spi.c: Fix memory leak in function pony_init_spi
......................................................................
pony_spi.c: Fix memory leak in function pony_init_spi
I found the issue by running scan-build. Memory leaked was caused as
data variable wasn't deallocated in some error cases where the
function returned without deallocating it. After making the changes the
issue was no longer appeared in scan-build.
Change-Id: I7910db94f63693e7f131836d4963e88cfdbec301
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M pony_spi.c
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/62724/6
--
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: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Martin Roth.
Felix Singer 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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Comment-Date: Fri, 11 Mar 2022 01:08:33 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment