Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34591 )
Change subject: vc/cavium/bdk/libbdk-hal: Fix eye data memory leak ......................................................................
vc/cavium/bdk/libbdk-hal: Fix eye data memory leak
This function can capture and allocate its own eye data, so in that case set need_free to true so it is freed at the end.
Change-Id: I63ca6d743e6610d3e3ab6bd7b0356aabdfa6f784 Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1393969 --- M src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c 1 file changed, 5 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/34591/1
diff --git a/src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c b/src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c index f7d631f..5416b73 100644 --- a/src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c +++ b/src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c @@ -362,9 +362,12 @@ bdk_error("Failed to allocate space for eye\n"); return -1; } - if (bdk_qlm_eye_capture(node, qlm, qlm_lane, eye_data)) - return -1; + if (bdk_qlm_eye_capture(node, qlm, qlm_lane, eye_data)) { + free(eye_data); + return -1; + } eye = eye_data; + need_free = 1; }
/* Calculate the max eye width */
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34591 )
Change subject: vc/cavium/bdk/libbdk-hal: Fix eye data memory leak ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34591/1/src/vendorcode/cavium/bdk/l... File src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c:
https://review.coreboot.org/c/coreboot/+/34591/1/src/vendorcode/cavium/bdk/l... PS1, Line 365: { Looks like this vendorcode uses a different coding style... I'd be consistent with it.
https://review.coreboot.org/c/coreboot/+/34591/1/src/vendorcode/cavium/bdk/l... PS1, Line 366: eye_data Very minor: cast this as "(void*)eye_data" to be consistent with the free() call at the end of the function. Not sure if this makes any meaningful difference.
Hello Patrick Rudolph, David Hendricks, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34591
to look at the new patch set (#2).
Change subject: vc/cavium/bdk/libbdk-hal: Fix eye data memory leak ......................................................................
vc/cavium/bdk/libbdk-hal: Fix eye data memory leak
This function can capture and allocate its own eye data, so in that case set need_free to true so it is freed at the end.
Change-Id: I63ca6d743e6610d3e3ab6bd7b0356aabdfa6f784 Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1393969 --- M src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c 1 file changed, 5 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/34591/2
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34591 )
Change subject: vc/cavium/bdk/libbdk-hal: Fix eye data memory leak ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34591/1/src/vendorcode/cavium/bdk/l... File src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c:
https://review.coreboot.org/c/coreboot/+/34591/1/src/vendorcode/cavium/bdk/l... PS1, Line 365: {
Looks like this vendorcode uses a different coding style... I'd be consistent with it.
Done
https://review.coreboot.org/c/coreboot/+/34591/1/src/vendorcode/cavium/bdk/l... PS1, Line 366: eye_data
Very minor: cast this as "(void*)eye_data" to be consistent with the free() call at the end of the f […]
The (void *) cast at the end is needed to get rid of the const qualifier, which isn't needed here.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34591 )
Change subject: vc/cavium/bdk/libbdk-hal: Fix eye data memory leak ......................................................................
Patch Set 2: Code-Review+2
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34591 )
Change subject: vc/cavium/bdk/libbdk-hal: Fix eye data memory leak ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34591/1/src/vendorcode/cavium/bdk/l... File src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c:
https://review.coreboot.org/c/coreboot/+/34591/1/src/vendorcode/cavium/bdk/l... PS1, Line 366: eye_data
The (void *) cast at the end is needed to get rid of the const qualifier, which isn't needed here.
Done
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34591 )
Change subject: vc/cavium/bdk/libbdk-hal: Fix eye data memory leak ......................................................................
vc/cavium/bdk/libbdk-hal: Fix eye data memory leak
This function can capture and allocate its own eye data, so in that case set need_free to true so it is freed at the end.
Change-Id: I63ca6d743e6610d3e3ab6bd7b0356aabdfa6f784 Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1393969 Reviewed-on: https://review.coreboot.org/c/coreboot/+/34591 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org --- M src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c 1 file changed, 5 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved
diff --git a/src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c b/src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c index f7d631f..b9552d4 100644 --- a/src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c +++ b/src/vendorcode/cavium/bdk/libbdk-hal/bdk-qlm.c @@ -362,9 +362,12 @@ bdk_error("Failed to allocate space for eye\n"); return -1; } - if (bdk_qlm_eye_capture(node, qlm, qlm_lane, eye_data)) - return -1; + if (bdk_qlm_eye_capture(node, qlm, qlm_lane, eye_data)) { + free(eye_data); + return -1; + } eye = eye_data; + need_free = 1; }
/* Calculate the max eye width */
-
Angel Pons (Code Review) -
Jacob Garber (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)