Date: Wed, 12 Jan 2022 08:50:19 +0100 From: =?UTF-8?Q?Jan_Kokem=c3=bcller?= <jan.kokemueller@gmail.com> To: Stefan Esser <se@FreeBSD.org>, Mark Millard <marklmi@yahoo.com> Cc: bugs@openbsd.org, freebsd-current <freebsd-current@freebsd.org>, Baptiste Daroussin <bapt@FreeBSD.org> Subject: Re: UBSAN report for main [so: 14] /usr/bin/whatis: non-zero (48) and zero offsets from null pointer in qsort.c Message-ID: <80e1f514-c0b3-cf79-ea6f-8c62cb1db386@gmail.com> In-Reply-To: <7babd754-6dab-223a-7bfd-ff06f10c71e2@FreeBSD.org> References: <A4577E70-AB32-450F-A3F6-A2B42B09A1B3.ref@yahoo.com> <A4577E70-AB32-450F-A3F6-A2B42B09A1B3@yahoo.com> <35333abc-9d4a-4b78-586d-1e869df4f9d4@FreeBSD.org> <BEFB4665-F32B-4AA0-BE4A-5ABB8B973012@yahoo.com> <7babd754-6dab-223a-7bfd-ff06f10c71e2@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11.01.22 22:08, Stefan Esser wrote:
> 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.
The UB happens in this line, when "a == NULL" and "n == 0", right?
for (pm = (char *)a + es; pm < (char *)a + n * es; pm += es)
This is arithmetic on a pointer (the NULL pointer) which is not part of an
array, which is UB.
Then, wouldn't "if (__predict_false(n == 0))" be more appropriate than checking
for "a == NULL" here? Testing for "a == NULL" might suppress UBSAN warnings of
valid bugs, i.e. when "qsort" is called with "a == NULL" and "n != 0". In that
case UBSAN _should_ trigger.
UBSAN should not trigger when n == 0, though. At least, when "a" does point to
a valid array. But what about the case of "a == NULL && n == 0"? Is that deemed
UB? It looks like at least FreeBSD's "qsort_s" implementation says it's legal.
a != NULL (pointing to valid array), n != 0 -> "normal" case, no UB
a != NULL (pointing to valid array), n == 0 -> should not trigger UB, and
doesn't in the current
implementation
a == NULL, n == 0 -> should not trigger UB?
(debatable)
So if "a == NULL && n == 0" was deemed legal, then there would be no bug in
"mansearch.c", right?
-Jan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?80e1f514-c0b3-cf79-ea6f-8c62cb1db386>