Attention is currently required from: Felix Singer, Nico Huber, Light, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62747
to look at the new patch set (#15).
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.
Similar reasoning for variables hbuf, hdrs, tbuf in function
prob_spi_sfdp.
So allocate them using calloc to initialize them to zeroes. Also, copy
"FLASHROM BUG!" to detect future bugs.
Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M flashrom.c
M sfdp.c
2 files changed, 9 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/62747/15
--
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: 15
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Paul Menzel, Angel Pons, Light.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62725 )
Change subject: libflashrom.c: Fix unintialized value passed to function
......................................................................
Patch Set 17: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62725/comment/7c4e90a2_2aaba647
PS15, Line 11: value
> I think the current explanation should be satisfactory.
Oh, sorry for not responding! I thought I replied on this one.
My main concern was that a value is value and I thought the variable can be uninitialized, but not the value.
Yes, it's nitpicking. I really forgot responding. However, thanks for changing!
--
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: 17
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: 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: 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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Tue, 29 Mar 2022 09:47:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Nico Huber, Paul Menzel, Angel Pons, Anastasia Klimchuk.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62867 )
Change subject: ichspi: Unify timeouts across all SPI operations to 30s
......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS5:
> Oh I was referring to previous code,
>
> ich_hwseq_block_erase had `uint32_t timeout = 5000 * 1000;`
> and for example in
> ich_hwseq_write had `uint16_t timeout = 100 * 60;`
> tow different values.
>
> and now this patch makes the same timeout everywhere. So I was wondering why there were different values before?
It was to meet the SPI operational timeout recommendation by the vendor datasheet (example: winbond spiflash W25Q256JV-DTR specification, table 9.7).
--
To view, visit https://review.coreboot.org/c/flashrom/+/62867
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b
Gerrit-Change-Number: 62867
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: 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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
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: Tue, 29 Mar 2022 08:38:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Anastasia Klimchuk.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62747 )
Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
......................................................................
Patch Set 14:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62747/comment/695eaf7c_2512cfc7
PS9, Line 11: when later used in need_erase could result in undefined behaviour.
> That would depend much on the kind of bug we are trying to catch, […]
I have copied the "FLASHROM BUG!" to oldcontents. Not sure if I have to do the same for curcontents, hbuf, hdrs and tbuf also.
--
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: 14
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 29 Mar 2022 08:34:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Light, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62747
to look at the new patch set (#14).
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.
Similar reasoning for variables hbuf, hdrs, tbuf in function
prob_spi_sfdp.
So allocate them using calloc to initialize them to zeroes. Also, copy
"FLASHROM BUG!" to detect future bugs.
Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M flashrom.c
M sfdp.c
2 files changed, 9 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/62747/14
--
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: 14
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Paul Menzel, Edward O'Callaghan, Angel Pons.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62726
to look at the new patch set (#15).
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/15
--
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: 15
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: 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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Light, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62747
to look at the new patch set (#13).
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.
Similar reasoning for variables hbuf, hdrs, tbuf in function
prob_spi_sfdp.
So allocate them using calloc to initialize them to zeroes. Also, copy
"FLASHROM BUG!" to detect future bugs.
Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M flashrom.c
M sfdp.c
2 files changed, 10 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/62747/13
--
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: 13
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62725 )
Change subject: libflashrom.c: Fix unintialized value passed to function
......................................................................
Patch Set 17:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62725/comment/527b9404_f11e27ee
PS15, Line 11: value
> Yeah, I agree with Nico. […]
I think the current explanation should be satisfactory.
--
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: 17
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: 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: 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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 29 Mar 2022 08:08:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Light.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62725
to look at the new patch set (#17).
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
variable (which contains a garbage value) is passed to
flashrom_layout_release. Thus initialize it with NULL. For completeness,
also initialize dump_layout with NULL.
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/17
--
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: 17
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: 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: 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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset