Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/46551 )
Change subject: it87spi.c: Prevent use-after-free bug ......................................................................
it87spi.c: Prevent use-after-free bug
The memory for the `param` string is aliased by `dualbiosindex_suffix`. Therefore, only free the former when there are no more uses of either.
Change-Id: I79f18f6077c77c0cbb8bfa431e17f9b079f11c95 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M it87spi.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/46551/1
diff --git a/it87spi.c b/it87spi.c index e34f5e8..79b4448 100644 --- a/it87spi.c +++ b/it87spi.c @@ -134,12 +134,13 @@ char *dualbiosindex_suffix; errno = 0; long chip_index = strtol(param, &dualbiosindex_suffix, 0); - free(param); if (errno != 0 || *dualbiosindex_suffix != '\0' || chip_index < 0 || chip_index > 1) { msg_perr("DualBIOS: Invalid chip index requested - choose 0 or 1.\n"); + free(param); exit_conf_mode_ite(port); return 1; } + free(param); if (chip_index != (tmp & 1)) { msg_pdbg("DualBIOS: Previous chip index: %d\n", tmp & 1); sio_write(port, 0xEF, (tmp & 0xFE) | chip_index);
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46551 )
Change subject: it87spi.c: Prevent use-after-free bug ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/flashrom/+/46551/1/it87spi.c File it87spi.c:
https://review.coreboot.org/c/flashrom/+/46551/1/it87spi.c@137 PS1, Line 137: errno != 0 Not sure if it's worth to mention, but we also checked `errno` of `free()` here by accident.
And probably for another patch: Checking `errno` isn't needed at all, as `*param != '\0' && *dualbiosindex_suffix == '\0'` implies success.
Hello build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/46551
to look at the new patch set (#2).
Change subject: it87spi.c: Prevent use-after-free bug ......................................................................
it87spi.c: Prevent use-after-free bug
The memory for the `param` string is aliased by `dualbiosindex_suffix`. Moreover, `errno` could have been modified by the call to `free()`. Therefore, only free the former when there are no more uses of either.
Change-Id: I79f18f6077c77c0cbb8bfa431e17f9b079f11c95 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M it87spi.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/46551/2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/46551 )
Change subject: it87spi.c: Prevent use-after-free bug ......................................................................
it87spi.c: Prevent use-after-free bug
The memory for the `param` string is aliased by `dualbiosindex_suffix`. Moreover, `errno` could have been modified by the call to `free()`. Therefore, only free the former when there are no more uses of either.
Change-Id: I79f18f6077c77c0cbb8bfa431e17f9b079f11c95 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/46551 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M it87spi.c 1 file changed, 2 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/it87spi.c b/it87spi.c index e34f5e8..79b4448 100644 --- a/it87spi.c +++ b/it87spi.c @@ -134,12 +134,13 @@ char *dualbiosindex_suffix; errno = 0; long chip_index = strtol(param, &dualbiosindex_suffix, 0); - free(param); if (errno != 0 || *dualbiosindex_suffix != '\0' || chip_index < 0 || chip_index > 1) { msg_perr("DualBIOS: Invalid chip index requested - choose 0 or 1.\n"); + free(param); exit_conf_mode_ite(port); return 1; } + free(param); if (chip_index != (tmp & 1)) { msg_pdbg("DualBIOS: Previous chip index: %d\n", tmp & 1); sio_write(port, 0xEF, (tmp & 0xFE) | chip_index);
-
Angel Pons (Code Review) -
Nico Huber (Code Review)