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. 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: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49741/comment/72da94ff_27eaa553 PS1, Line 7: helpers.c: optimize strndup > Thanks a bunch for the rewrite Angel, I couldn't understand the initial msg either and was going to […] About adding a unit-test for this function: Maybe, but it is only used for MinGW. Moreover, I doubt unit-tests can catch undefined behavior. I don't mind if someone writes a unit-test for it, but I think there's better candidates for unit-testing. -- 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: Paul Menzel <paulepanter(a)users.sourceforge.net> 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-Comment-Date: Wed, 20 Jan 2021 15:19:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Xiang Wang <merle(a)hardenedlinux.org> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> 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 Edward O'Callaghan (Code Review) 20 Jan '21

20 Jan '21

Attention is currently required from: Xiang Wang, Stefan Reinauer. Edward O'Callaghan 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 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49741/comment/4b69cc63_bde38df3 PS1, Line 7: helpers.c: optimize strndup > really thankful Thanks a bunch for the rewrite Angel, I couldn't understand the initial msg either and was going to come back with a comment on some questions. However your feedback was super constructive! Thanks Xiang Wang for all these patches! What do you think about adding a unit-test for this kind of stuff? -- 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: Paul Menzel <paulepanter(a)users.sourceforge.net> 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-Comment-Date: Wed, 20 Jan 2021 15:04:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Xiang Wang <merle(a)hardenedlinux.org> 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 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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2021