Page MenuHomeFreeBSD

bspatch: add integer overflow checks
ClosedPublic

Authored by emaste on Sep 14 2016, 9:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 25, 7:34 PM
Unknown Object (File)
Jan 13 2024, 3:53 AM
Unknown Object (File)
Dec 27 2023, 7:56 PM
Unknown Object (File)
Dec 26 2023, 1:12 AM
Unknown Object (File)
Dec 23 2023, 2:56 AM
Unknown Object (File)
Sep 12 2023, 5:20 AM
Unknown Object (File)
Jul 24 2023, 7:39 PM
Unknown Object (File)
Jun 17 2023, 11:40 PM
Subscribers

Details

Summary

Introduce a new add_off_t static function that exits with an error message if there's an overflow, otherwise returns their sum. Use this when adding values obtained from the input patch.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emaste retitled this revision from to bspatch: add integer overflow checks.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added reviewers: allanjude, delphij, cperciva, kib.
usr.bin/bsdiff/bspatch/bspatch.c
78 ↗(On Diff #20356)

Is this variant of the code so much better than explicit overflow checks below that it warrants coding the same check twice ? You need to do the explicit check anyway, so why bother ?

What can be done, instead, is to add e.g. sys/cdefs.h macro ADD_OF_CHECKED(a,b,result) and make this functionality generally useful, to avoid open-coding the same fragment several times. typeof() macro or generic C11 facility would allow to make that type-neutral.

usr.bin/bsdiff/bspatch/bspatch.c
78 ↗(On Diff #20356)

Or may be sys/param.h is better place.

usr.bin/bsdiff/bspatch/bspatch.c
78 ↗(On Diff #20356)

We could do away with the bzctrllen > OFF_MAX - HEADER_SIZE, bzctrllen + HEADER_SIZE > OFF_MAX - bzdatalen and bzctrllen + HEADER_SIZE > OFF_MAX - bzdatalen checks if we were so inclined.

This approach is harder to get wrong (by missing an explicit overflow check), and is resilient to future changes in variable types.

This revision is now accepted and ready to land.Apr 21 2018, 3:26 AM

Looks good to me in principal.

usr.bin/bsdiff/bspatch/bspatch.c
70 ↗(On Diff #20356)

Maybe inline?

rebase, add inline as suggested by @delphij

This revision now requires review to proceed.Aug 16 2019, 6:45 PM
This revision is now accepted and ready to land.Aug 16 2019, 8:47 PM
This revision was automatically updated to reflect the committed changes.