Commit 10af8e45 authored by Stefan Eßer's avatar Stefan Eßer
Browse files

fread.c: fix undefined behavior

A case of undefined behavior in __fread() has been detected by UBSAN
and reported by Mark Millard:

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

While being benign (the NULL pointer is later passed to memcpy() with
a length argument of 0), this issue causes in the order of 600 Kyua
test cases to fail on systems running a world built with WITH_UBSAN
and WITH_ASAN.

The undefined behavior can be prevented by skipping operations that
have no effect for r == 0. Mark Millard has suggested to only skip
this code segment if fp->_p == NULL, but I have verified that for the
case of r == 0 no further argument checking is performed on the
addresses passed to memcpy() and thus no bugs are hidden from the
sanitizers due to the simpler condition chosen.

Reported by:	Mark Millard (marklmi@yahoo.com)
Tested by:	Mark Millard (marklmi@yahoo.com)
Differential Revision:	https://reviews.freebsd.org/D33903
MFC after:	2 weeks
parent c8dafefa
......@@ -129,11 +129,13 @@ __fread(void * __restrict buf, size_t size, size_t count, FILE * __restrict fp)
}
while (resid > (r = fp->_r)) {
(void)memcpy((void *)p, (void *)fp->_p, (size_t)r);
fp->_p += r;
/* fp->_r = 0 ... done in __srefill */
p += r;
resid -= r;
if (r != 0) {
(void)memcpy((void *)p, (void *)fp->_p, (size_t)r);
fp->_p += r;
/* fp->_r = 0 ... done in __srefill */
p += r;
resid -= r;
}
if (__srefill(fp)) {
/* no more input: return partial result */
return ((total - resid) / size);
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment