Date: Tue, 11 Jan 2022 22:08:50 +0100 From: Stefan Esser <se@FreeBSD.org> To: 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: <7babd754-6dab-223a-7bfd-ff06f10c71e2@FreeBSD.org> In-Reply-To: <BEFB4665-F32B-4AA0-BE4A-5ABB8B973012@yahoo.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--------------so9aXA5pmnmGs0EZ0xCJDgxM
Content-Type: multipart/mixed; boundary="------------3PXzrn6hY9M6pIwW3w6IdnxA";
protected-headers="v1"
From: Stefan Esser <se@FreeBSD.org>
To: Mark Millard <marklmi@yahoo.com>
Cc: bugs@openbsd.org, freebsd-current <freebsd-current@freebsd.org>,
Baptiste Daroussin <bapt@FreeBSD.org>
Message-ID: <7babd754-6dab-223a-7bfd-ff06f10c71e2@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
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>
In-Reply-To: <BEFB4665-F32B-4AA0-BE4A-5ABB8B973012@yahoo.com>
--------------3PXzrn6hY9M6pIwW3w6IdnxA
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
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 parameter=
s
>> 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 =3D maxres =3D 0;=09
>> if (res !=3D NULL)
>> *res =3D NULL;
>>
>> If no match is found, these values are unchanged at line 223, where re=
s
>> 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 !=3D NULL)
>> + if (res !=3D NULL && *res !=3D NULL)
>> qsort(*res, cur, sizeof(struct manpage), manpage_compare);
>> if (chdir_status && getcwd_status && chdir(buf) =3D=3D -1)
>> warn("%s", buf);
>>
>> (File name as in OpenBSD, it is contrib/mandoc/mansearch.c in FreeBSD.=
)
>=20
> Cool. Thanks.
>=20
> (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.)
>=20
> =3D=3D=3D
> Mark Millard
> marklmi at yahoo.com
>=20
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 =3D=3D NULL))
+ return;
loop:
swap_cnt =3D 0;
if (n < 7) {
This would also work to prevent the NULL pointer arithmetik for
ports that might also path a =3D=3D NULL and n =3D=3D 0 in certain cases.=
I'll apply this patch tomorrow, if there are no objections.
Regards, STefan
--------------3PXzrn6hY9M6pIwW3w6IdnxA--
--------------so9aXA5pmnmGs0EZ0xCJDgxM
Content-Type: application/pgp-signature; name="OpenPGP_signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="OpenPGP_signature"
-----BEGIN PGP SIGNATURE-----
wsB5BAABCAAjFiEEo3HqZZwL7MgrcVMTR+u171r99UQFAmHd8eIFAwAAAAAACgkQR+u171r99UQr
ZAf+K0z2y4yiqFHAMP0tV/OBU7De/LL5J+Yjzx941LcPUBNt4T9mclLV9w6aIijP+mUJ7UG5qD+D
ULyxRQcQ3qkDND/8oc0prceo0fzi8fmeK1Jq54+jwR4VX+siWzq3+hnKYV8YBmSbmAcrRpnd9YgT
ZaCJvkPddKMXzQan/Ke2so2I4NFiOxLKDkm2jHu+idov9EsZOTUYkxvOHyGBPQ55CZ131PrkbMnE
qnQjwHUg06S/SIJFxIAWmLOy9ctmsgizFA4hmysqZA+dZ1Zk22I9kxPxgCnM0RCpbFbHW6tD7iwk
6hiwZjCzR6L6ucoztK7+iaaec2AtR9JMZ48fTXg1NA==
=P3g4
-----END PGP SIGNATURE-----
--------------so9aXA5pmnmGs0EZ0xCJDgxM--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?7babd754-6dab-223a-7bfd-ff06f10c71e2>