Re: UBSAN report for main [so: 14] /usr/bin/whatis: non-zero (48) and zero offsets from null pointer in qsort.c
- Reply: Jan_Kokemüller : "Re: UBSAN report for main [so: 14] /usr/bin/whatis: non-zero (48) and zero offsets from null pointer in qsort.c"
- In reply to: Mark Millard : "Re: UBSAN report for main [so: 14] /usr/bin/whatis: non-zero (48) and zero offsets from null pointer in qsort.c"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 11 Jan 2022 21:08:50 UTC
Am 11.01.22 um 21:08 schrieb Mark Millard: > On 2022-Jan-11, at 05:19, Stefan Esser <se@freebsd.org> wrote: [...] >> The undefined behavior is caused by insufficient checking of parameters >> in mansearch.c. >> >> As part of the initializations performed at the start of mansearch(), >> the variables cur and *res are initialized to 0 resp. NULL: >> >> cur = maxres = 0; >> if (res != NULL) >> *res = NULL; >> >> If no match is found, these values are unchanged at line 223, where res >> is checked to be non-NULL, but then *res is passed to qsort() and that >> is still NULL. >> >> Suggested fix (also attached to avoid white-space issues): >> >> --- usr.bin/mandoc/mansearch.c >> +++ usr.bin/mandoc/mansearch.c >> @@ -220,7 +220,7 @@ >> if (cur && search->firstmatch) >> break; >> } >> - if (res != NULL) >> + if (res != NULL && *res != NULL) >> qsort(*res, cur, sizeof(struct manpage), manpage_compare); >> if (chdir_status && getcwd_status && chdir(buf) == -1) >> warn("%s", buf); >> >> (File name as in OpenBSD, it is contrib/mandoc/mansearch.c in FreeBSD.) > > Cool. Thanks. > > (But I'm not a committer so someone else > will have to deal with doing an update to > the file in git --and likely MFC'ing it.) > > === > Mark Millard > marklmi at yahoo.com > I have submitted a bug report to our upstream (OpenBSD), but the issue could also be fixed (or rather undefined behavior prevented) by a simple patch that makes qsort() detect the NULL pointer: diff --git a/lib/libc/stdlib/qsort.c b/lib/libc/stdlib/qsort.c index 5016fff7895f..51c41e802330 100644 --- a/lib/libc/stdlib/qsort.c +++ b/lib/libc/stdlib/qsort.c @@ -108,6 +108,8 @@ local_qsort(void *a, size_t n, size_t es, cmp_t *cmp, void *thunk) int cmp_result; int swap_cnt; + if (__predict_false(a == NULL)) + return; loop: swap_cnt = 0; if (n < 7) { This would also work to prevent the NULL pointer arithmetik for ports that might also path a == NULL and n == 0 in certain cases. I'll apply this patch tomorrow, if there are no objections. Regards, STefan