Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39975 )
Change subject: ft2232_spi.c: Improve handling of static buffer ......................................................................
ft2232_spi.c: Improve handling of static buffer
If `buf` became NULL because of an error, subsequent calls to the `ft2232_spi_send_command` function with a smaller buffer size will result in a null pointer dereference. Add an additional null check before using `buf` to prevent that. Moreover, use `size_t` for the `bufsize` and `oldbufsize` variables, as it's what `realloc` uses.
Change-Id: Idc4237ddca94c42ce2a930e6d00fd2d14e4f125c Signed-off-by: Angel Pons th3fanbus@gmail.com --- M ft2232_spi.c 1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/39975/1
diff --git a/ft2232_spi.c b/ft2232_spi.c index 1a5b2fe..84aebb3 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -468,8 +468,8 @@ static unsigned char *buf = NULL; /* failed is special. We use bitwise ops, but it is essentially bool. */ int i = 0, ret = 0, failed = 0; - int bufsize; - static int oldbufsize = 0; + size_t bufsize; + static size_t oldbufsize = 0;
if (writecnt > 65536 || readcnt > 65536) return SPI_INVALID_LENGTH; @@ -477,7 +477,7 @@ /* buf is not used for the response from the chip. */ bufsize = max(writecnt + 9, 260 + 9); /* Never shrink. realloc() calls are expensive. */ - if (bufsize > oldbufsize) { + if (!buf || bufsize > oldbufsize) { buf = realloc(buf, bufsize); if (!buf) { msg_perr("Out of memory!\n");
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39975 )
Change subject: ft2232_spi.c: Improve handling of static buffer ......................................................................
Patch Set 3: Code-Review+2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39975 )
Change subject: ft2232_spi.c: Improve handling of static buffer ......................................................................
Patch Set 3: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/39975 )
Change subject: ft2232_spi.c: Improve handling of static buffer ......................................................................
ft2232_spi.c: Improve handling of static buffer
If `buf` became NULL because of an error, subsequent calls to the `ft2232_spi_send_command` function with a smaller buffer size will result in a null pointer dereference. Add an additional null check before using `buf` to prevent that. Moreover, use `size_t` for the `bufsize` and `oldbufsize` variables, as it's what `realloc` uses.
Change-Id: Idc4237ddca94c42ce2a930e6d00fd2d14e4f125c Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/39975 Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: Edward O'Callaghan quasisec@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M ft2232_spi.c 1 file changed, 3 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified HAOUAS Elyes: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved
diff --git a/ft2232_spi.c b/ft2232_spi.c index 520eb6e..9f4c7f0 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -468,8 +468,8 @@ static unsigned char *buf = NULL; /* failed is special. We use bitwise ops, but it is essentially bool. */ int i = 0, ret = 0, failed = 0; - int bufsize; - static int oldbufsize = 0; + size_t bufsize; + static size_t oldbufsize = 0;
if (writecnt > 65536 || readcnt > 65536) return SPI_INVALID_LENGTH; @@ -477,7 +477,7 @@ /* buf is not used for the response from the chip. */ bufsize = max(writecnt + 9, 260 + 9); /* Never shrink. realloc() calls are expensive. */ - if (bufsize > oldbufsize) { + if (!buf || bufsize > oldbufsize) { buf = realloc(buf, bufsize); if (!buf) { msg_perr("Out of memory!\n");
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
HAOUAS Elyes (Code Review)