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

Change in flashrom[master]: flashrom.8.tmpl: document lspcon_i2c_spi
by Edward O'Callaghan (Code Review) 29 Mar '22

29 Mar '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/63104 ) Change subject: flashrom.8.tmpl: document lspcon_i2c_spi ...................................................................... flashrom.8.tmpl: document lspcon_i2c_spi This programmer operates much the same as realtek_mst_i2c_spi, so the I2C options are moved to a new section describing both programmers and a short description is added for this programmer itself. Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: I9ccb9694fdea29e68f062cc049efc0204917a139 Reviewed-on: https://review.coreboot.org/c/flashrom/+/63104 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.8.tmpl 1 file changed, 22 insertions(+), 11 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Nikolai Artemiev: Looks good to me, but someone else must approve diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index 92dff68..bf8960f 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -395,6 +395,8 @@ .sp .BR "* realtek_mst_i2c_spi" " (for SPI flash ROMs attached to Realtek DisplayPort hubs accessible through I2C)" .sp +.BR "* lspcon_i2c_spi" " (for SPI flash ROMs attached to Parade Technologies LSPCONs)" +.sp Some programmers have optional or mandatory parameters which are described in detail in the .B PROGRAMMER-SPECIFIC INFORMATION @@ -1417,21 +1419,25 @@ If the passed frequency is not supported by the adapter the nearest lower supported frequency will be used. .SS -.BR "realtek_mst_i2c_spi " programmer +.BR "realtek_mst_i2c_spi " and " lspcon_i2c_spi " programmers .IP -This programmer supports SPI flash programming for chips attached to Realtek -DisplayPort MST hubs, themselves accessed through I2C (tunneling SPI flash -commands through the MST hub's I2C connection with the host). - -The I2C bus on which the hub is reachable must be specified by either a device -path with the \fBdevpath\fP option: +These programmers tunnel SPI commands through I2C-connected devices. The I2C +bus over which communication occurs must be specified either by device path +with the \fBdevpath\fP option: .sp .B " flashrom \-p realtek_mst_i2c_spi:devpath=/dev/i2c-8" .sp or by a bus number with the \fBbus\fP option, which implies a device path like /dev/i2c-N where N is the specified bus number: .sp -.B " flashrom \-p realtek_mst_i2c_spi:bus=8" +.B " flashrom \-p lspcon_i2c_spi:bus=8" + +.SS +.BR "realtek_mst_i2c_spi " programmer +.IP +This programmer supports SPI flash programming for chips attached to Realtek +DisplayPort MST hubs, themselves accessed through I2C (tunneling SPI flash +commands through the MST hub's I2C connection with the host). .TP .B In-system programming (ISP) mode .sp @@ -1456,7 +1462,12 @@ .B " flashrom -p realtek_mst_i2c_spi:bus=0,enter-isp=1,reset-mcu=0 -E" .br .B " flashrom -p realtek_mst_i2c_spi:bus=0,enter-isp=0,reset-mcu=1 -w new.bin" -.sp +.SS +.BR "lspcon_i2c_spi " programmer +.IP +This programmer supports SPI flash programming for chips attached to Parade +Technologies DisplayPort-to-HDMI level shifter/protocol converters (LSPCONs). +Communication to the SPI flash is tunneled through the LSPCON over I2C. .SH EXAMPLES To back up and update your BIOS, run @@ -1535,8 +1546,8 @@ .B ogp needs PCI configuration space read access and raw memory access. .sp -.B realtek_mst_i2c_spi -needs userspace access to the selected I2C bus. +.BR realtek_mst_i2c_spi " and " lspcon_i2c_spi +need userspace access to the selected I2C bus. .sp On OpenBSD, you can obtain raw access permission by setting .B "securelevel=-1" 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/63104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9ccb9694fdea29e68f062cc049efc0204917a139 Gerrit-Change-Number: 63104 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(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-MessageType: merged

1 0

Change in flashrom[master]: flashrom.8.tmpl: document realtek_mst_i2c_spi
by Edward O'Callaghan (Code Review) 29 Mar '22

29 Mar '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/63103 ) Change subject: flashrom.8.tmpl: document realtek_mst_i2c_spi ...................................................................... flashrom.8.tmpl: document realtek_mst_i2c_spi This programmer was undocumented. Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: Idde5a8de014fe84c4a472f8fbfd3562350997d39 Reviewed-on: https://review.coreboot.org/c/flashrom/+/63103 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.8.tmpl 1 file changed, 45 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Nikolai Artemiev: Looks good to me, but someone else must approve diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index 3e4bc5e..92dff68 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -393,6 +393,8 @@ .sp .BR "* stlinkv3_spi" " (for SPI flash ROMs attached to STMicroelectronics STLINK V3 devices)" .sp +.BR "* realtek_mst_i2c_spi" " (for SPI flash ROMs attached to Realtek DisplayPort hubs accessible through I2C)" +.sp Some programmers have optional or mandatory parameters which are described in detail in the .B PROGRAMMER-SPECIFIC INFORMATION @@ -1415,6 +1417,46 @@ If the passed frequency is not supported by the adapter the nearest lower supported frequency will be used. .SS +.BR "realtek_mst_i2c_spi " programmer +.IP +This programmer supports SPI flash programming for chips attached to Realtek +DisplayPort MST hubs, themselves accessed through I2C (tunneling SPI flash +commands through the MST hub's I2C connection with the host). + +The I2C bus on which the hub is reachable must be specified by either a device +path with the \fBdevpath\fP option: +.sp +.B " flashrom \-p realtek_mst_i2c_spi:devpath=/dev/i2c-8" +.sp +or by a bus number with the \fBbus\fP option, which implies a device path like +/dev/i2c-N where N is the specified bus number: +.sp +.B " flashrom \-p realtek_mst_i2c_spi:bus=8" +.TP +.B In-system programming (ISP) mode +.sp +The \fBreset-mcu\fP and \fBenter-isp\fP options provide control over device +mode changes, where each can be set to 0 or 1 to enable or disable the +corresponding mode transition. + +\fBenter-isp\fP defaults to 1, and if enabled will issue commands to the MST +hub when beginning operation to put it into ISP mode. + +\fBreset-mcu\fP defaults to 0, and if enabled will issue a reset command to +the MST hub on programming completion, causing it to exit ISP mode and to +reload its own firmware from flash. + +The hub must be in ISP mode for SPI flash access to be possible, so it is +usually only useful to disable \fBenter-isp\fP if an earlier invocation avoided +resetting it on completion. For instance, to erase the flash and +rewrite it with the contents of a file without resetting in between (which +could render it nonfunctional if attempting to load firmware from a blank +flash): +.sp +.B " flashrom -p realtek_mst_i2c_spi:bus=0,enter-isp=1,reset-mcu=0 -E" +.br +.B " flashrom -p realtek_mst_i2c_spi:bus=0,enter-isp=0,reset-mcu=1 -w new.bin" +.sp .SH EXAMPLES To back up and update your BIOS, run @@ -1493,6 +1535,9 @@ .B ogp needs PCI configuration space read access and raw memory access. .sp +.B realtek_mst_i2c_spi +needs userspace access to the selected I2C bus. +.sp On OpenBSD, you can obtain raw access permission by setting .B "securelevel=-1" in 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/63103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idde5a8de014fe84c4a472f8fbfd3562350997d39 Gerrit-Change-Number: 63103 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(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-MessageType: merged

1 0

Change in flashrom[master]: flashrom.8.tmpl: Clarify man entries for -w/-v/-x
by Edward O'Callaghan (Code Review) 29 Mar '22

29 Mar '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/62794 ) Change subject: flashrom.8.tmpl: Clarify man entries for -w/-v/-x ...................................................................... flashrom.8.tmpl: Clarify man entries for -w/-v/-x This change adds follow up changes to the man page: - Explain (-) argument for -w/-v operations - Expand on region name handling of -x operation Also updates cli_classic.c to match with --help output. BUG=b:224364316 Change-Id: I0cba593da3926c8587027789f4e1e89a2329ca7f Signed-off-by: Daniel Campello <campello(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/62794 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M cli_classic.c M flashrom.8.tmpl 2 files changed, 13 insertions(+), 7 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index b3930d2..0b6e79c 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -47,9 +47,9 @@ printf(" -h | --help print this help text\n" " -R | --version print version (release)\n" " -r | --read <file> read flash and save to <file>\n" - " -w | --write <file|-> write <file> or the content provided\n" + " -w | --write (<file>|-) write <file> or the content provided\n" " on the standard input to flash\n" - " -v | --verify <file|-> verify flash against <file>\n" + " -v | --verify (<file>|-) verify flash against <file>\n" " or the content provided on the standard input\n" " -E | --erase erase flash memory\n" " -V | --verbose more verbose output\n" diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index fe2d9d6..3e4bc5e 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -82,11 +82,13 @@ .BR <file> . If the file already exists, it will be overwritten. .TP -.B "\-w, \-\-write <file>" +.B "\-w, \-\-write (<file>|-)" Write .B <file> -into flash ROM. This will first automatically -.B erase +into flash ROM. If +.B - +is provided instead, contents will be read from stdin. This will first automatically + B erase the chip, then write to it. .sp In the process the chip is also read several times. First an in-memory backup @@ -124,16 +126,20 @@ .BR internal programmer. It may be enabled by default in this case in the future. .TP -.B "\-v, \-\-verify <file>" +.B "\-v, \-\-verify (<file>|-)" Verify the flash ROM contents against the given .BR <file> . +If +.BR - +is provided instead, contents will be written to the stdout. .TP .B "\-E, \-\-erase" Erase the flash ROM chip. .TP .B "\-x, \-\-extract" Extract every region defined on the layout from flash ROM chip to a -file with the same name as the extracted region. +file with the same name as the extracted region (replacing spaces with +underscores). .TP .B "\-V, \-\-verbose" More verbose output. This option can be supplied multiple times -- To view, visit https://review.coreboot.org/c/flashrom/+/62794 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0cba593da3926c8587027789f4e1e89a2329ca7f Gerrit-Change-Number: 62794 Gerrit-PatchSet: 6 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
by Light (Code Review) 29 Mar '22

29 Mar '22

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

1 0

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

29 Mar '22

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

1 0

Change in flashrom[master]: ichspi: Unify timeouts across all SPI operations to 30s
by Subrata Banik (Code Review) 29 Mar '22

29 Mar '22

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

1 0

Change in flashrom[master]: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
by Light (Code Review) 29 Mar '22

29 Mar '22

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

1 0

Change in flashrom[master]: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
by Light (Code Review) 29 Mar '22

29 Mar '22

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

1 0

Change in flashrom[master]: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
by Light (Code Review) 29 Mar '22

29 Mar '22

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

1 0

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

29 Mar '22

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

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022