Change in coreboot[master]: commonlib/region: Fix up overflow check in region_is_subregion()
Hello Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/34892
to review the following change.
Change subject: commonlib/region: Fix up overflow check in region_is_subregion() ......................................................................
commonlib/region: Fix up overflow check in region_is_subregion()
region_is_subregion() checks whether the size of the inner region is larger than the size of the outer region... which isn't really necessary because we're already checking the starts and ends of both regions. Maybe this was added to ensure the inner region doesn't overflow? But it's not guaranteed to catch that in all cases. Replace it with a proper overflow check.
Change-Id: I9e442053584a479a323c1fa1c0591934ff83eb10 Signed-off-by: Julius Werner jwerner@chromium.org --- M src/commonlib/region.c 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/34892/1
diff --git a/src/commonlib/region.c b/src/commonlib/region.c index 541a125..b5858f9 100644 --- a/src/commonlib/region.c +++ b/src/commonlib/region.c @@ -27,10 +27,10 @@ if (region_offset(c) < region_offset(p)) return 0;
- if (region_sz(c) > region_sz(p)) + if (region_end(c) > region_end(p)) return 0;
- if (region_end(c) > region_end(p)) + if (region_end(c) < region_offset(c)) return 0;
return 1;
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34892 )
Change subject: commonlib/region: Fix up overflow check in region_is_subregion() ......................................................................
Patch Set 1: Code-Review+2
Julius Werner has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34892 )
Change subject: commonlib/region: Fix up overflow check in region_is_subregion() ......................................................................
commonlib/region: Fix up overflow check in region_is_subregion()
region_is_subregion() checks whether the size of the inner region is larger than the size of the outer region... which isn't really necessary because we're already checking the starts and ends of both regions. Maybe this was added to ensure the inner region doesn't overflow? But it's not guaranteed to catch that in all cases. Replace it with a proper overflow check.
Change-Id: I9e442053584a479a323c1fa1c0591934ff83eb10 Signed-off-by: Julius Werner jwerner@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/34892 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/commonlib/region.c 1 file changed, 2 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/commonlib/region.c b/src/commonlib/region.c index 541a125..b5858f9 100644 --- a/src/commonlib/region.c +++ b/src/commonlib/region.c @@ -27,10 +27,10 @@ if (region_offset(c) < region_offset(p)) return 0;
- if (region_sz(c) > region_sz(p)) + if (region_end(c) > region_end(p)) return 0;
- if (region_end(c) > region_end(p)) + if (region_end(c) < region_offset(c)) return 0;
return 1;
-
Furquan Shaikh (Code Review) -
Julius Werner (Code Review)