Page MenuHomeFreeBSD

Fix undefined behavior in fread()

Authored by se on Jan 15 2022, 2:54 PM.



Mark Millard has reported another case of undefined behavior in a central library function, detected on a system built with UBSAN and ASAN:

/usr/main-src/lib/libc/stdio/fread.c:133:10: runtime error: applying zero offset to null pointer

The pointer calculated in this way is then passed as source address to memcpy(), but with a length parameter of 0 in all observed cases.

Test Plan

Apply patch and verify that read operations work as before, but there are no further warnings from the sanitizers ...

Diff Detail

R10 FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

se requested review of this revision.Jan 15 2022, 2:54 PM

The code looks good to me and is strongly related to what I've been recently using personally for chrooting
into a WITH_UBSAN= WITH_ASAN= world and running the kyua tests, avoiding the particular UBSAN stderr
outputs from the NULL+0 messing up 600+ test results.

I do not see any reason that I should need to update to using the patch and making another kyua run for an
approval here: so approved.

(I will eventually be doing that update and run, however. Likely sometime later today in my time zone.)

This revision is now accepted and ready to land.Jan 15 2022, 9:09 PM
This revision was automatically updated to reflect the committed changes.

I have updated to the system being based on (line split for readability):

# uname -apKU
FreeBSD amd64_ZFS 14.0-CURRENT FreeBSD 14.0-CURRENT #30
main-n252475-e76c0108990b-dirty: Sat Jan 15 21:18:14 PST 2022
amd64 amd64 1400047 1400047

so it has the committed material (and does not have my prior personal workaround). Based on also using:

env SH_DISABLE_VFORK= ASAN_OPTIONS=detect_container_overflow=0 chroot /usr/obj/DESTDIRs/main-amd64-xSAN-chroot/

for the (also updated) chroot area, I'm now getting:

===> Summary
Results read from /usr/home/root/.kyua/store/results.usr_tests.20220116-063311-263708.db
Test cases: 6840 total, 894 skipped, 29 expected failures, 142 broken, 303 failed
Total time: 3132.706s

with no fread.c reports. In my first runs, before I'd learned some about what to control,
there were over 1200 failures reported (not limited to fread.c ones).

The commit worked well at eliminating the fread.c UndefinedBehavior reports.

[There is still a lot of vfork usage to avoid somehow, but the SH_DISABLE_VFORK= use stopped
/bin/sh from contributing to the vfork usage, which was much of the vfork usage.]