Attention is currently required from: Nico Huber, Light.
Hello build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62725
to look at the new patch set (#2).
Change subject: libflashrom.c: Fix unintialized value passed to function
......................................................................
libflashrom.c: Fix unintialized value passed to function
In function flash_layout_read_from_ifd variable chip_layout remains
uninitialized if prepare_flash_access returns false. This uninitialized
value is passed to flashrom_layout_release later.
Change-Id: Iacbd7bf9cdf897cc2a732c1dc6568845a4ab804d
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M libflashrom.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/62725/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/62725
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iacbd7bf9cdf897cc2a732c1dc6568845a4ab804d
Gerrit-Change-Number: 62725
Gerrit-PatchSet: 2
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Light.
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62726
to look at the new patch set (#4).
Change subject: serprog.c: Avoid calling memcpy with NULL pointer arguments
......................................................................
serprog.c: Avoid calling memcpy with NULL pointer arguments
In function sp_stream_buffer_op, the variable parms might be NULL when
passed to memcpy. Although, since parmlen is also 0 at that time I
don't think it would make much difference. Still, add a NULL check before calling memcpy to be safe.
Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M serprog.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/62726/4
--
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: 4
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: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61275 )
Change subject: hwaccess_x86_io: refactor rget_io_perms()
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/61275
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42
Gerrit-Change-Number: 61275
Gerrit-PatchSet: 8
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Thu, 10 Mar 2022 10:02:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Light.
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62726
to look at the new patch set (#3).
Change subject: serprog.c: Avoid calling memcpy with NULL pointer arguments
......................................................................
serprog.c: Avoid calling memcpy with NULL pointer arguments
In function sp_stream_buffer_op, the variable parms might be NULL when
passed to memcpy. Although, since parmlen is also 0 at that time I
don't think it would make much difference. Still, add a NULL check before calling memcpy to be safe.
Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M serprog.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/62726/3
--
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: 3
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: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Light.
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62726
to look at the new patch set (#2).
Change subject: serprog.c: Fix passed null value when expectingn nonull
......................................................................
serprog.c: Fix passed null value when expectingn nonull
In function sp_stream_buffer_op, the variable parms might be NULL when
passed to memcpy. Although, since parmlen is also 0 at that time I
don't think it would make much difference. Still, add a NULL check before calling memcpy to be safe.
Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M serprog.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/62726/2
--
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: 2
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: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Light.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62724 )
Change subject: pony_spi.c: Fixed memory leak in function pony_init_spi
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62724/comment/9b5f79a9_9673bb6a
PS1, Line 11:
You can add information on how you tested this patch. For example, you found this by running scan-build, so you can say that you ran scan-build before and after and the issue disappeared (if this is true of course).
Patchset:
PS1:
Welcome Aarya! You will need to read all the comments carefully, fix all them and upload new version of this patch (new patchset). If you don't understand the comment, it is totally fine to ask for clarification, and what does the comment mean. Good luck! :)
--
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: 1
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Thu, 10 Mar 2022 09:46:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61854 )
Change subject: ichspi.c: Check SPI Cycle In-Progress prior start HW Seq
......................................................................
Patch Set 17:
(1 comment)
Patchset:
PS17:
> I know I can react much too harsh sometimes (it's just frustrating to see […]
Thanks Nico,
I just wanted to follow up here quickly to ensure that messaging does not get mixed up and this is going to get followed up on! Tomorrow is some company-wide day off.
The short answer just for the moment is that Subrata and I were discussing today if a possible unit-test using HECI command set that could perhaps reliably exploit the issue and how it is remediated by this patch can be contrived to make this more concrete.
I also followed up with Anastasia to help assist in making these changes smoother and she has provided some very useful insight to.
More to come but I just wanted to follow up with something here ASAP. Leaving as unresolved.
--
To view, visit https://review.coreboot.org/c/flashrom/+/61854
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib9265cc20513fd00f32f8fa22e28c312903ca484
Gerrit-Change-Number: 61854
Gerrit-PatchSet: 17
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Alex Levin <levinale(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Thu, 10 Mar 2022 09:37:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Light.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62724 )
Change subject: pony_spi.c: Fixed memory leak in function pony_init_spi
......................................................................
Patch Set 1:
(6 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62724/comment/9628eccf_f3b982d7
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.org/Development_Guidelines#Commit_message
File pony_spi.c:
https://review.coreboot.org/c/flashrom/+/62724/comment/8a9dfab8_dbaf898a
PS1, Line 164: free(data);
We use tabs for indentation, and a tab is 8 characters wide. Please refer to https://flashrom.org/Development_Guidelines#Coding_style
Besides the indentation, this is correct. The shutdown function hasn't been registered (see comment on line 167), so `data` leaks.
https://review.coreboot.org/c/flashrom/+/62724/comment/7260c2c7_1e1d0a77
PS1, Line 167:
Once execution reaches this point, we know that `register_shutdown(pony_spi_shutdown, data)` has been called successfully. This means flashrom will automatically execute the `pony_spi_shutdown()` function before exiting, which in turn calls `free(data)`. So, there's no need to `free(data)` from this point onwards.
https://review.coreboot.org/c/flashrom/+/62724/comment/b28a00e2_9407e9d0
PS1, Line 182: free(data);
This isn't needed because the shutdown function has been registered, which will run `free(data)` already. Adding `free(data)` here would result in a double-free bug.
https://review.coreboot.org/c/flashrom/+/62724/comment/63515572_6b943e63
PS1, Line 248: free(data);
This isn't needed either, see my comment on line 182.
https://review.coreboot.org/c/flashrom/+/62724/comment/c1f1bcca_174a24a2
PS1, Line 255: free(data);
This won't work. The `register_spi_bitbang_master()` function stores a copy of the `data` pointer and uses it as argument for the `spi_data` parameter of the functions referenced by the `bitbang_spi_master_pony` struct. The memory is released by the `pony_spi_shutdown()` function.
--
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: 1
Gerrit-Owner: Light <aarya.chaumal(a)gmail.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: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Thu, 10 Mar 2022 09:34:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment