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

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

10 Mar '22

Attention is currently required from: 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 (#5). 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 deallocatiing 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/5 -- 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: 5 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-MessageType: newpatchset

1 0

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

10 Mar '22

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 (#7). 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/7 -- 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: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: libflashrom.c: Fix unintialized value passed to function
by Light (Code Review) 10 Mar '22

10 Mar '22

Attention is currently required from: Paul Menzel, Angel Pons, 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 (#8). 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/8 -- 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: 8 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: 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

1 0

Change in flashrom[master]: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits
by Nico Huber (Code Review) 10 Mar '22

10 Mar '22

Attention is currently required from: Angel Pons, Light, Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62733 ) Change subject: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: Heh, a third solution comes to mind. Sorry if this is getting too theoretical. Just ignore this comment at your convenience. ;) Prove that it can't happen and ignore the tool. I think that is possible. My reasoning: * If `bp_bit_count == 0`, then `bp == 0` and the error path isn't taken. * If `bp_bit_count == 1`, then either `bp == 0` or `bp == 1 == bp_max` and the error path isn't taken. -- 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: 4 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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 10 Mar 2022 18:16:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits
by Nico Huber (Code Review) 10 Mar '22

10 Mar '22

Attention is currently required from: Angel Pons, Light, Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62733 ) Change subject: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62733/comment/ca7449dd_562a8093 PS2, Line 12: Still the bug is not fixed :( > I don't think it would work as the max value that bp_max can take is 15 since the array bits->bp is […] The types can't help us because an underflow or a negative `bp_max - 2` both would result in undefined behavior. From C11 about the shift operators: "If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined." As far as I can see, the bug is only a theoretical one that depends on the arguments passed to this function (i.e. `bits->bp_bit_count`). I see two potential solutions: 1. Either add an assert() that would provide a contract how this function should be called, e.g. on the top of the function: assert(bits->bp_bit_count > 1); 2. Defensive programming. We could add an `if` that returns an error if this happens. Personally, I'm much a fan of 1. But I am not sure if it will be accepted by scan-build. -- 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: 4 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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 10 Mar 2022 17:59:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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]: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits
by Light (Code Review) 10 Mar '22

10 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62733 to look at the new patch set (#4). Change subject: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits ...................................................................... writeprotect_ranges.c: Use 1UL if left shifting more than 32bits size_t is of size 64 bits, while 1 (int) is of 32 bits. Left shifting 1 by more than 32 will lead to shift-count-overflow bug. So use 1UL or 1ULL to make 1 of type unsigned long or unsigned long long respectively. Change-Id: I2766ef8e34f7121dad746e5f32a835d480ae1cad Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M writeprotect_ranges.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/62733/4 -- 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: 4 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: libflashrom.c: Fix unintialized value passed to function
by Nico Huber (Code Review) 10 Mar '22

10 Mar '22

Attention is currently required from: Paul Menzel, Angel Pons, Light. Nico Huber 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 7: Code-Review+2 (7 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62725/comment/bfdc40d5_741005ae PS1, Line 7: fixed > Fix Done https://review.coreboot.org/c/flashrom/+/62725/comment/54413eda_7495a33f PS1, Line 9: y > t Done https://review.coreboot.org/c/flashrom/+/62725/comment/54b944ce_a9d554d5 PS1, Line 9: In function flash_layouy_read_from_ifd variable chip_layout remains uninitialized if prepare_flash_access returns > Please wrap commit message lines at 72 characters, c.f. https://flashrom. […] Done https://review.coreboot.org/c/flashrom/+/62725/comment/206c968f_98897b36 PS1, Line 10: flase > typo: false Done https://review.coreboot.org/c/flashrom/+/62725/comment/8c8872ea_e8a03f3d PS1, Line 10: unintialzed > 2 typos: uninitialized Done Patchset: PS7: Hi Aarya, nice patch! There's one tiny typo left in the commit message I think. File libflashrom.c: https://review.coreboot.org/c/flashrom/+/62725/comment/eabe3039_b4e23023 PS1, Line 445: struct flashrom_layout *dump_layout=0, *chip_layout=0; > Since these are pointers, I'd use NULL. […] Done -- 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: 7 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: 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: Thu, 10 Mar 2022 17:32:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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]: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits
by Light (Code Review) 10 Mar '22

10 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62733 ) Change subject: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62733/comment/54d8f9a0_92fec78a PS2, Line 12: Still the bug is not fixed :( > I wonder if casting the `1` to `size_t` would work: […] I don't think it would work as the max value that bp_max can take is 15 since the array bits->bp is of length MAX_BP_BITS which is 4. The error arises when bp_max takes values lower than 2. Then bp_max-2 becomes very large due to underflow error since size_t is unsigned type. Any suggestions to solve this error? -- 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: 3 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 10 Mar 2022 16:52:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits
by Light (Code Review) 10 Mar '22

10 Mar '22

Attention is currently required from: Nico Huber, Light. Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62733 to look at the new patch set (#3). Change subject: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits ...................................................................... writeprotect_ranges.c: Use 1UL if left shifting more than 32bits size_t is of size 64 bits, while 1 (int) is of 32 bits. Left shifting 1 by more than 32 will lead to shift-count-overflow bug. So use 1UL or 1ULL to make 1 of type unsigned long or unsigned long long respectively. Change-Id: I2766ef8e34f7121dad746e5f32a835d480ae1cad Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M writeprotect_ranges.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/62733/3 -- 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: 3 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits
by Angel Pons (Code Review) 10 Mar '22

10 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: Use 1UL if left shifting more than 32bits ...................................................................... Patch Set 2: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62733/comment/3cf38b51_f8c30c2a PS2, Line 9: size_t is of size 64 bits Not always, it could be 32 bits wide on 32-bit machines https://review.coreboot.org/c/flashrom/+/62733/comment/84c69a7e_16ac5511 PS2, Line 12: Still the bug is not fixed :( I wonder if casting the `1` to `size_t` would work: (size_t)1 << foo File tags: PS2: Did you mean to add this file? -- 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: 2 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 10 Mar 2022 15:25:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022