Re: kern/52383 (Crash when connecting specific USB devices)

[Pierre Pronchery <khorben%defora.org@localhost>]

The following reply was made to PR kern/52383; it has been noted by GNATS.

From: Pierre Pronchery <khorben%defora.org@localhost>
To: Nick Hudson <nick.hudson%gmx.co.uk@localhost>, gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/52383 (Crash when connecting specific USB devices)
Date: Fri, 8 Dec 2017 14:41:30 +0100

 This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
 --spW01H4GlEpuAfWB1NO6tRAM4bXrsmR65
 Content-Type: multipart/mixed; boundary="g5UqfEA986TrAIdGj5J4kQgMVAbU9w5F3";
  protected-headers="v1"
 From: Pierre Pronchery <khorben%defora.org@localhost>
 To: Nick Hudson <nick.hudson%gmx.co.uk@localhost>, gnats-bugs%NetBSD.org@localhost
 Message-ID: <320310b9-fc9e-40bd-bf81-d8a28f6da795%defora.org@localhost>
 Subject: Re: kern/52383 (Crash when connecting specific USB devices)
 References: <pr-kern-52383%gnats.netbsd.org@localhost>
  <20170709141201.84A691097E%tungsten.defora.rom@localhost>
  <20171104164715.ECC217A1EC%mollari.NetBSD.org@localhost>
  <163bba19-4a61-f88e-3deb-1f453cba97ba%gmx.co.uk@localhost>
 In-Reply-To: <163bba19-4a61-f88e-3deb-1f453cba97ba%gmx.co.uk@localhost>
 
 --g5UqfEA986TrAIdGj5J4kQgMVAbU9w5F3
 Content-Type: multipart/mixed;
  boundary="------------FFB1CA1FAED165B0238928E4"
 Content-Language: en-US
 
 This is a multi-part message in MIME format.
 --------------FFB1CA1FAED165B0238928E4
 Content-Type: text/plain; charset=utf-8
 Content-Transfer-Encoding: quoted-printable
 
 			Hi Nick,
 
 On 22/11/2017 14:07, Nick Hudson wrote:
 > =C2=A0> panic: kernel diagnostic assertion "requested_size > 0" failed:=
  file
 > "[...]/src/sys/kern/subr_kmem.c", line 261
 >=20
 > Should be handled better by the diff
 
 I found the real problem: until a specific USB configuration is chosen
 by the user, the device provides no interface
 (usb_config_descriptor_t.bNumInterfaces). The crash happens therefore a
 few lines after your change, here:
 
 >  725         /* Allocate and fill interface data. */
 >  726         nifc =3D cdp->bNumInterface;                              =
    =20
 >  727         dev->ud_ifaces =3D kmem_alloc(nifc * sizeof(struct usbd_in=
 terface),
 >  728             KM_SLEEP);
 In any case, it looks sensible to protect ourselves against malicious
 USB devices by being generally more defensive while probing.
 
 What do you think of the patch attached?
 
 Thanks for your help!
 --=20
 khorben
 
 --------------FFB1CA1FAED165B0238928E4
 Content-Type: text/x-patch;
  name="patch-usb_subr.diff"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment;
  filename="patch-usb_subr.diff"
 
 Index: sys/dev/usb/usb_subr.c
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 RCS file: /cvsroot/src/sys/dev/usb/usb_subr.c,v
 retrieving revision 1.221
 diff -p -u -r1.221 usb_subr.c
 --- sys/dev/usb/usb_subr.c	28 Oct 2017 00:37:12 -0000	1.221
 +++ sys/dev/usb/usb_subr.c	8 Dec 2017 13:38:43 -0000
 @@ -609,6 +609,10 @@ usbd_set_config_index(struct usbd_device
  		return err;
  	}
  	len =3D UGETW(cd.wTotalLength);
 +	if (len =3D=3D 0) {
 +		DPRINTF("empty short descriptor", 0, 0, 0, 0);
 +		return USBD_INVAL;
 +	}
  	cdp =3D kmem_alloc(len, KM_SLEEP);
 =20
  	/* Get the full descriptor.  Try a few times for slow devices. */
 @@ -635,6 +639,11 @@ usbd_set_config_index(struct usbd_device
  		err =3D usbd_get_bos_desc(dev, index, &bd);
  		if (!err) {
  			int blen =3D UGETW(bd.wTotalLength);
 +			if (blen =3D=3D 0) {
 +				DPRINTF("empty bos descriptor", 0, 0, 0, 0);
 +				err =3D USBD_INVAL;
 +				goto bad;
 +			}
  			bdp =3D kmem_alloc(blen, KM_SLEEP);
 =20
  			/* Get the full desc */
 @@ -724,6 +733,11 @@ usbd_set_config_index(struct usbd_device
 =20
  	/* Allocate and fill interface data. */
  	nifc =3D cdp->bNumInterface;
 +	if (nifc =3D=3D 0) {
 +		DPRINTF("no interfaces", 0, 0, 0, 0);
 +		err =3D USBD_INVAL;
 +		goto bad;
 +	}
  	dev->ud_ifaces =3D kmem_alloc(nifc * sizeof(struct usbd_interface),
  	    KM_SLEEP);
  	DPRINTFN(5, "dev=3D%#jx cdesc=3D%#jx", (uintptr_t)dev, (uintptr_t)cdp,
 
 --------------FFB1CA1FAED165B0238928E4--
 
 --g5UqfEA986TrAIdGj5J4kQgMVAbU9w5F3--
 
 --spW01H4GlEpuAfWB1NO6tRAM4bXrsmR65
 Content-Type: application/pgp-signature; name="signature.asc"
 Content-Description: OpenPGP digital signature
 Content-Disposition: attachment; filename="signature.asc"
 
 -----BEGIN PGP SIGNATURE-----
 
 iQJHBAEBCAAxFiEEjPEp1wC4bxBrX8svMDjL25iGlwMFAloqlo0THGtob3JiZW5A
 ZGVmb3JhLm9yZwAKCRAwOMvbmIaXA2BLD/4qoBLkVhZYRILvmj5WjTY1BH392jcW
 9kUwbkudC8qCbR7jYfqs9pVe2h7Lj95zcNwvg+gz2krnGOBQcvEEcYv5J10RZDQp
 kyGmAxbn/BlJQ/014+1Dnz8BT5LT+ay79th6UF+QrgJ/I4l59PVZqIHHNFIoWpMy
 fLBScvNLgbCqr6RpIUva/GLh27MIjknt/kg6UhyPIDaBsiGGPtWJShS+u7l4AsS0
 fFhJ6yC2LZObEMO4fXJ447wSKb2z+KbmV7fZGsXAhPqIyB3P4W0Eu+JdZ69+MxTO
 MNF58ApVbwuwl5hD2fFdYd27T7yYTHxp42ohsWCbZeiD3E1RAPP98ALENxzhmei/
 bddK1u8xvN7/GZTnVVaCV+XHZjHBy1mKPUUCVlwMvxYm3gamM8KiwVTBl1AQvwlz
 mqdLajyrlIxpyOoqI1yXtsAxMt/OgG25bopAzjUcGN7RR5wHJmBM8XZkZ9ocY2aQ
 yk7POKvUE1wYWYqWdQdYnelzwQgDp4/e1ZSmqYU8Qca4+nizZ08LzqtdJTSA/Dsi
 aD9J15K/Q0Aqj7mK/W1KoOAe35li57lfJ4uyvpflUUqigK5keo2wxr5UZFZShrkZ
 AqWXhMG5zvNcGoHpSLEpm0Ge0JI4JKB9EHtGw9FdcoTVh+jmoFGyhBkXwVcrfVsO
 Q0Gisv0NgJvT+A==
 =Q2Qk
 -----END PGP SIGNATURE-----
 
 --spW01H4GlEpuAfWB1NO6tRAM4bXrsmR65--