Yu-Ping Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38987 )
Change subject: libpayload: cbgfx: Fix potential overflowing expression ......................................................................
libpayload: cbgfx: Fix potential overflowing expression
BRANCH=none BUG=none TEST=none
Change-Id: Icd37a6abc01d9fcbcf54525d47b15c9930a9b9fb Signed-off-by: Yu-Ping Wu yupingso@google.com --- M payloads/libpayload/drivers/video/graphics.c 1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/38987/1
diff --git a/payloads/libpayload/drivers/video/graphics.c b/payloads/libpayload/drivers/video/graphics.c index 8cb984b8..9494de3 100644 --- a/payloads/libpayload/drivers/video/graphics.c +++ b/payloads/libpayload/drivers/video/graphics.c @@ -349,8 +349,8 @@ /* Use 64 bits to avoid overflow */ int32_t x, y; uint64_t yy; - const uint64_t rrx = r.x * r.x, rry = r.y * r.y; - const uint64_t ssx = s.x * s.x, ssy = s.y * s.y; + const uint64_t rrx = (uint64_t)r.x * r.x, rry = (uint64_t)r.y * r.y; + const uint64_t ssx = (uint64_t)s.x * s.x, ssy = (uint64_t)s.y * s.y; x_begin = 0; x_end = 0; for (y = r.y - 1; y >= 0; y--) { @@ -358,7 +358,7 @@ * The inequality is valid in the beginning of each iteration: * y^2 + x_end^2 < r^2 */ - yy = y * y; + yy = (uint64_t)y * y; /* Check yy/ssy + xx/ssx < 1 */ while (yy * ssx + x_begin * x_begin * ssy < ssx * ssy) x_begin++;
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38987 )
Change subject: libpayload: cbgfx: Fix potential overflowing expression ......................................................................
Patch Set 1:
Coverity found some issues: CID 1419491: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38987 )
Change subject: libpayload: cbgfx: Fix potential overflowing expression ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38987/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38987/1//COMMIT_MSG@8 PS1, Line 8: Found by what program? Do you have example values causing the overflow?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38987 )
Change subject: libpayload: cbgfx: Fix potential overflowing expression ......................................................................
Patch Set 1: Code-Review+2
Thanks for taking care of these!
Patch Set 1: Coverity found some issues: CID 1419491: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
We usually add that to the commit message, below Signed-off-by:
Found-by: Coverity Scan #1419491
Hello Julius Werner, Hung-Te Lin, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38987
to look at the new patch set (#2).
Change subject: libpayload: cbgfx: Fix potential overflowing expression ......................................................................
libpayload: cbgfx: Fix potential overflowing expression
BRANCH=none BUG=none TEST=none
Change-Id: Icd37a6abc01d9fcbcf54525d47b15c9930a9b9fb Signed-off-by: Yu-Ping Wu yupingso@google.com Found-by: Coverity Scan #1419491 --- M payloads/libpayload/drivers/video/graphics.c 1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/38987/2
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38987 )
Change subject: libpayload: cbgfx: Fix potential overflowing expression ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38987/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38987/1//COMMIT_MSG@8 PS1, Line 8:
Found by what program? Do you have example values causing the overflow?
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38987 )
Change subject: libpayload: cbgfx: Fix potential overflowing expression ......................................................................
libpayload: cbgfx: Fix potential overflowing expression
BRANCH=none BUG=none TEST=none
Change-Id: Icd37a6abc01d9fcbcf54525d47b15c9930a9b9fb Signed-off-by: Yu-Ping Wu yupingso@google.com Found-by: Coverity Scan #1419491 Reviewed-on: https://review.coreboot.org/c/coreboot/+/38987 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Georgi pgeorgi@google.com --- M payloads/libpayload/drivers/video/graphics.c 1 file changed, 3 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/payloads/libpayload/drivers/video/graphics.c b/payloads/libpayload/drivers/video/graphics.c index 8cb984b8..9494de3 100644 --- a/payloads/libpayload/drivers/video/graphics.c +++ b/payloads/libpayload/drivers/video/graphics.c @@ -349,8 +349,8 @@ /* Use 64 bits to avoid overflow */ int32_t x, y; uint64_t yy; - const uint64_t rrx = r.x * r.x, rry = r.y * r.y; - const uint64_t ssx = s.x * s.x, ssy = s.y * s.y; + const uint64_t rrx = (uint64_t)r.x * r.x, rry = (uint64_t)r.y * r.y; + const uint64_t ssx = (uint64_t)s.x * s.x, ssy = (uint64_t)s.y * s.y; x_begin = 0; x_end = 0; for (y = r.y - 1; y >= 0; y--) { @@ -358,7 +358,7 @@ * The inequality is valid in the beginning of each iteration: * y^2 + x_end^2 < r^2 */ - yy = y * y; + yy = (uint64_t)y * y; /* Check yy/ssy + xx/ssx < 1 */ while (yy * ssx + x_begin * x_begin * ssy < ssx * ssy) x_begin++;
-
Patrick Georgi (Code Review) -
Paul Menzel (Code Review)
-
Yu-Ping Wu (Code Review)