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

Change in flashrom[master]: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow
by Light (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: 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/+/62764 to look at the new patch set (#15). Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow ...................................................................... ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow Unsigned types show undefined behaviour if they are subtracted by a value greater than their own (mostly it wraps to the max value). Using this value for left shifting could be even more dangerous. Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M ich_descriptors.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/62764/15 -- To view, visit https://review.coreboot.org/c/flashrom/+/62764 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39 Gerrit-Change-Number: 62764 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: 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

1 0

Change in flashrom[master]: pony_spi.c: Extract out get_params to simplify init
by Light (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63130 ) Change subject: pony_spi.c: Extract out get_params to simplify init ...................................................................... Patch Set 1: (1 comment) File pony_spi.c: https://review.coreboot.org/c/flashrom/+/63130/comment/7820752e_f4c5eabb PS1, Line 164: free(data); Why isn't data freed in the patch? Wouldn't this lead to memory leak. -- To view, visit https://review.coreboot.org/c/flashrom/+/63130 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I364febc05c870683cbad114583762b0c006f4bac Gerrit-Change-Number: 63130 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Mar 2022 10:42:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow ...
by Light (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Edward O'Callaghan, Angel Pons. Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 ) Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow error ...................................................................... Patch Set 14: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62764/comment/17cd4d7a_a2d36b05 PS14, Line 501: for (j = 0; j < (size_t)min(num_regions, 12); j++) > Maybe these `12`'s should be a `#define MAX_NUM_REGIONS 12` or at the very least a constant after li […] I don't think it's the max number of regions as num_regions can be > 12. Can you suggest any other name for the macro. -- To view, visit https://review.coreboot.org/c/flashrom/+/62764 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39 Gerrit-Change-Number: 62764 Gerrit-PatchSet: 14 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 28 Mar 2022 10:29:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

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

28 Mar '22

Attention is currently required from: Felix Singer, Paul Menzel, Edward O'Callaghan, 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 14: (1 comment) File serprog.c: https://review.coreboot.org/c/flashrom/+/62726/comment/24d80284_3d424861 PS14, Line 288: m > `calloc(1, 1 + parmlen);` so that sp[1] is zeroed in the case of `(parmlen:=0, params:=NULL)`. Does it help to calloc there? -- 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: 14 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-Comment-Date: Mon, 28 Mar 2022 09:55:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow ...
by Edward O'Callaghan (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Angel Pons, Light. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 ) Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow error ...................................................................... Patch Set 14: Code-Review+1 (4 comments) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62764/comment/332894d1_2a900d04 PS14, Line 501: for (j = 0; j < (size_t)min(num_regions, 12); j++) Maybe these `12`'s should be a `#define MAX_NUM_REGIONS 12` or at the very least a constant after line 489. https://review.coreboot.org/c/flashrom/+/62764/comment/2f096e3b_81b9061e PS14, Line 505: for (j = 12; j < num_regions; j++) ditto https://review.coreboot.org/c/flashrom/+/62764/comment/31c31bf3_827fe455 PS14, Line 507: 12 ditto https://review.coreboot.org/c/flashrom/+/62764/comment/1691e479_8c73d6e8 PS14, Line 508: 12 ditto -- To view, visit https://review.coreboot.org/c/flashrom/+/62764 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39 Gerrit-Change-Number: 62764 Gerrit-PatchSet: 14 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: 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: Mon, 28 Mar 2022 09:51:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

28 Mar '22

Attention is currently required from: Felix Singer, Paul Menzel, Angel Pons, Light. Edward O'Callaghan 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 14: Code-Review+1 (1 comment) File serprog.c: https://review.coreboot.org/c/flashrom/+/62726/comment/54a2e94b_ef70552f PS14, Line 288: m `calloc(1, 1 + parmlen);` so that sp[1] is zeroed in the case of `(parmlen:=0, params:=NULL)`. -- 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: 14 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 28 Mar 2022 09:40:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

28 Mar '22

Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Light. Edward O'Callaghan 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 16: Code-Review+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: 16 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-Comment-Date: Mon, 28 Mar 2022 09:36:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c: Initialize structures
by Edward O'Callaghan (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Felix Singer, Nico Huber, Angel Pons, Light, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62763 ) Change subject: ich_descriptors.c: Initialize structures ...................................................................... Patch Set 10: Code-Review+2 (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62763/comment/20011bf3_0843bc37 PS7, Line 1364: int ret = read_ich_descriptors_from_dump(dump, len, &cs, &desc); > It could be a false-positive (in a project where tools like scan-build find little […] Not sure if it actually covers up a bug Nico, more that it would fail predictably in the same manner if there was a bug rather than - happens to have zero in memory this time. I get what your thinking here but types having default values at instantiation I wouldn't say shadows bugs. However, one could argue read_ich_descriptors_from_dump() should memset to zero before filling in fields so that the domain of the function is always well defined in desc. Depending on how one wishes to define their conventions of responsibilities. -- To view, visit https://review.coreboot.org/c/flashrom/+/62763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e5bc84c6199a0731db0a9c8ef56f1215686dab2 Gerrit-Change-Number: 62763 Gerrit-PatchSet: 10 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Mar 2022 09:34:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Edward O'Callaghan (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 6: (1 comment) Patchset: PS4: > No that was my fault, my dyslexic brain pattern matched the letters EHL to something else. […] Not sure what is actionable here Nico? The guess_ich_chipset_from_content() has now been implemented, there is subtle difference between them but not really much. -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 28 Mar 2022 09:26:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_mtd: Allow to open parition by name
by Arthur Heymans (Code Review) 28 Mar '22

28 Mar '22

Attention is currently required from: Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63135 ) Change subject: linux_mtd: Allow to open parition by name ...................................................................... Patch Set 1: (1 comment) File linux_mtd.c: https://review.coreboot.org/c/flashrom/+/63135/comment/9ba955ec_05b8e860 PS1, Line 464: extract_programmer_param("name"); Would it make sense to use this path when strtol path is not taken so that no new parameter is needed? Otherwise you might want some logic to make "dev" and "name" mutually exclusive? -- To view, visit https://review.coreboot.org/c/flashrom/+/63135 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I566b9809950cbf47fbb01ae89305641c4e259f68 Gerrit-Change-Number: 63135 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Mon, 28 Mar 2022 08:48:54 +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