Page MenuHomeFreeBSD

bspatch: add integer overflow checks
ClosedPublic

Authored by emaste on Sep 14 2016, 9:12 PM.

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

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

emaste updated this revision to Diff 20356.Sep 14 2016, 9:12 PM
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.
kib added inline comments.Sep 15 2016, 8:32 AM
usr.bin/bsdiff/bspatch/bspatch.c
72

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.

kib added inline comments.Sep 15 2016, 8:33 AM
usr.bin/bsdiff/bspatch/bspatch.c
72

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

emaste added inline comments.Sep 15 2016, 2:36 PM
usr.bin/bsdiff/bspatch/bspatch.c
72

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.

emaste added a subscriber: gordon.Apr 21 2018, 3:12 AM
allanjude accepted this revision.Apr 21 2018, 3:26 AM
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
64

Maybe inline?

emaste updated this revision to Diff 60902.Aug 16 2019, 6:45 PM

rebase, add inline as suggested by @delphij

This revision now requires review to proceed.Aug 16 2019, 6:45 PM
delphij accepted this revision.Aug 16 2019, 8:47 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.