Date: Sat, 15 Jan 2022 23:44:24 GMT From: =?utf-8?Q?Stefan E=C3=9Fer?= <se@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 10af8e45a898 - main - fread.c: fix undefined behavior Message-ID: <202201152344.20FNiOkH061564@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by se: URL: https://cgit.FreeBSD.org/src/commit/?id=10af8e45a89818754b80315539e167ae49599f17 commit 10af8e45a89818754b80315539e167ae49599f17 Author: Stefan Eßer <se@FreeBSD.org> AuthorDate: 2022-01-15 23:30:04 +0000 Commit: Stefan Eßer <se@FreeBSD.org> CommitDate: 2022-01-15 23:43:56 +0000 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 --- lib/libc/stdio/fread.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/libc/stdio/fread.c b/lib/libc/stdio/fread.c index 11f8d13f0caf..cafe86fe7961 100644 --- a/lib/libc/stdio/fread.c +++ b/lib/libc/stdio/fread.c @@ -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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202201152344.20FNiOkH061564>