January 2021 - flashrom-gerrit

Change in flashrom[master]: helpers.c: Fix undefined behavior in strndup()
by Angel Pons (Code Review) 20 Jan '21

20 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49741 ) Change subject: helpers.c: Fix undefined behavior in strndup() ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id34127024085879228626fbad59af03268ec5255 Gerrit-Change-Number: 49741 Gerrit-PatchSet: 2 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 20 Jan 2021 11:19:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: helpers.c: Fix undefined behavior in strndup()
by Xiang Wang (Code Review) 20 Jan '21

20 Jan '21

Attention is currently required from: Stefan Reinauer, Edward O'Callaghan, Angel Pons. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49741 ) Change subject: helpers.c: Fix undefined behavior in strndup() ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49741/comment/01d137e5_c3976887 PS1, Line 7: helpers.c: optimize strndup > An optimization? Really? […] really thankful -- To view, visit https://review.coreboot.org/c/flashrom/+/49741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id34127024085879228626fbad59af03268ec5255 Gerrit-Change-Number: 49741 Gerrit-PatchSet: 2 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 20 Jan 2021 11:13:27 +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]: helpers.c: Fix undefined behavior in strndup()
by Xiang Wang (Code Review) 20 Jan '21

20 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer, Edward O'Callaghan. Hello build bot (Jenkins), Stefan Reinauer, Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/49741 to look at the new patch set (#2). Change subject: helpers.c: Fix undefined behavior in strndup() ...................................................................... helpers.c: Fix undefined behavior in strndup() Using strlen() or strdup() inside strndup() is problematic: if the input string is not null-terminated, these functions can read past the end of the buffer, which triggers undefined behavior. Rewrite the function to never read past the provided `maxlen` bound. Change-Id: Id34127024085879228626fbad59af03268ec5255 Signed-off-by: Xiang Wang <merle(a)hardenedliux.org> --- M helpers.c 1 file changed, 9 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/49741/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/49741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id34127024085879228626fbad59af03268ec5255 Gerrit-Change-Number: 49741 Gerrit-PatchSet: 2 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: helpers.c: optimize strndup
by Angel Pons (Code Review) 20 Jan '21

20 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49741 ) Change subject: helpers.c: optimize strndup ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/49741/comment/79e75423_2f5715a6 PS1, Line 7: helpers.c: optimize strndup An optimization? Really? This change fixes potential undefined behavior. So, I'd rewrite the commit message as follows: helpers.c: Fix undefined behavior in strndup() Using strlen() or strdup() inside strndup() is problematic: if the input string is not null-terminated, these functions can read past the end of the buffer, which triggers undefined behavior. Rewrite the function to never read past the provided `maxlen` bound. https://review.coreboot.org/c/flashrom/+/49741/comment/b1860f77_35657b2d PS1, Line 10: has no feedback for a long time This is just one of the possible outcomes of undefined behavior. It could also result in a segmentation fault, or start reading registers from a MMIO window, or something else I didn't think of. -- To view, visit https://review.coreboot.org/c/flashrom/+/49741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id34127024085879228626fbad59af03268ec5255 Gerrit-Change-Number: 49741 Gerrit-PatchSet: 1 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 20 Jan 2021 10:55:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: helpers.c: optimize strndup
by Xiang Wang (Code Review) 20 Jan '21

20 Jan '21

Xiang Wang has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/49741 ) Change subject: helpers.c: optimize strndup ...................................................................... helpers.c: optimize strndup strndup is relative to strdup to prevent improper pointers from being passed in, so that the function has no feedback for a long time. But the old code used strlen in strndup. strlen may also have no feedback for a long time. This patch solves this problem. Change-Id: Id34127024085879228626fbad59af03268ec5255 Signed-off-by: Xiang Wang <merle(a)hardenedliux.org> --- M helpers.c 1 file changed, 9 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/49741/1 diff --git a/helpers.c b/helpers.c index c83cd2c..289848d 100644 --- a/helpers.c +++ b/helpers.c @@ -106,15 +106,16 @@ /* strndup is a POSIX function not present in MinGW */ char *strndup(const char *src, size_t maxlen) { - if (strlen(src) > maxlen) { - char *retbuf; - if ((retbuf = malloc(1 + maxlen)) != NULL) { - memcpy(retbuf, src, maxlen); - retbuf[maxlen] = '\0'; - } - return retbuf; + char *retbuf; + size_t len; + for (len = 0; len < maxlen; len++) + if (src[len] == '\0') + break; + if ((retbuf = malloc(1 + len)) != NULL) { + memcpy(retbuf, src, len); + retbuf[len] = '\0'; } - return strdup(src); + return retbuf; } #endif -- To view, visit https://review.coreboot.org/c/flashrom/+/49741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id34127024085879228626fbad59af03268ec5255 Gerrit-Change-Number: 49741 Gerrit-PatchSet: 1 Gerrit-Owner: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ft2232_spi.c: Generalized GPIOL pin control
by Angel Pons (Code Review) 20 Jan '21

20 Jan '21

Attention is currently required from: Alan Green, Xiang Wang, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49637 ) Change subject: ft2232_spi.c: Generalized GPIOL pin control ...................................................................... Patch Set 5: Code-Review+1 (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/49637/comment/1465d95b_d9e6a9e8 PS5, Line 7: Generalized GPIOL pin control Please make it a statement, e.g.: Generalize GPIOL pin control File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/49637/comment/0c4d245f_2253b00b PS5, Line 377: cs_bits pinlvl ? https://review.coreboot.org/c/flashrom/+/49637/comment/576cbe4d_a1a20494 PS5, Line 381: > Using the default value can work, but it is recommended to add the following code to make the meanin […] This would allow overriding the settings for some programmers -- To view, visit https://review.coreboot.org/c/flashrom/+/49637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab Gerrit-Change-Number: 49637 Gerrit-PatchSet: 5 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 20 Jan 2021 09:27:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Generalized GPIOL pin control
by Xiang Wang (Code Review) 20 Jan '21

20 Jan '21

Attention is currently required from: Alan Green, Paul Menzel, Angel Pons. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49637 ) Change subject: ft2232_spi.c: Generalized GPIOL pin control ...................................................................... Patch Set 5: (1 comment) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/49637/comment/d64482c8_b2ee5511 PS5, Line 384: Using the default value can work, but it is recommended to add the following code to make the meaning clearer. > pindir &= ~(1 << pin); -- To view, visit https://review.coreboot.org/c/flashrom/+/49637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab Gerrit-Change-Number: 49637 Gerrit-PatchSet: 5 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 20 Jan 2021 06:39:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Generalized GPIOL pin control
by Xiang Wang (Code Review) 20 Jan '21

20 Jan '21

Attention is currently required from: Alan Green, Paul Menzel, Angel Pons. Xiang Wang has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49637 ) Change subject: ft2232_spi.c: Generalized GPIOL pin control ...................................................................... Patch Set 5: Code-Review+1 (2 comments) Patchset: PS5: Very good, this is a more flexible option File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/49637/comment/554f1272_6cc41374 PS5, Line 381: Using the default value can work, but it is recommended to add the following code to make the meaning clearer. > cs_bits &= ~(1 << pin); -- To view, visit https://review.coreboot.org/c/flashrom/+/49637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1f2b3b968577e62e3c5b11bcdf4afe2de6eb84ab Gerrit-Change-Number: 49637 Gerrit-PatchSet: 5 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Xiang Wang <merle(a)hardenedlinux.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 20 Jan 2021 06:37:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer: remove unused noop_shutdown function
by Edward O'Callaghan (Code Review) 19 Jan '21

19 Jan '21

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/49631 ) Change subject: programmer: remove unused noop_shutdown function ...................................................................... programmer: remove unused noop_shutdown function Function appears to be vestigial. Signed-off-by: Alan Green <avg(a)google.com> Change-Id: I1b67223aed8be54b60771aa1b2d498836ed28060 Reviewed-on: https://review.coreboot.org/c/flashrom/+/49631 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M programmer.c 1 file changed, 0 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/programmer.c b/programmer.c index 5c841ef..bee60e3 100644 --- a/programmer.c +++ b/programmer.c @@ -17,12 +17,6 @@ #include "flash.h" #include "programmer.h" -/* No-op shutdown() for programmers which don't need special handling */ -int noop_shutdown(void) -{ - return 0; -} - /* Fallback map() for programmers which don't need special handling */ void *fallback_map(const char *descr, uintptr_t phys_addr, size_t len) { -- To view, visit https://review.coreboot.org/c/flashrom/+/49631 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b67223aed8be54b60771aa1b2d498836ed28060 Gerrit-Change-Number: 49631 Gerrit-PatchSet: 3 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: programmer: remove unused noop_shutdown function
by Alan Green (Code Review) 19 Jan '21

19 Jan '21

Attention is currently required from: Paul Menzel. Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49631 ) Change subject: programmer: remove unused noop_shutdown function ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49631/comment/4b20f813_b68b170b PS1, Line 9: vestigal > vestig*i*al Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/49631 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1b67223aed8be54b60771aa1b2d498836ed28060 Gerrit-Change-Number: 49631 Gerrit-PatchSet: 2 Gerrit-Owner: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 19 Jan 2021 03:22:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2021