usbd_set_config_index() is a complex function parsing unstrusted USB
descriptors. If a device presents itself with malformed descriptors
we should stop probing as soon as possible.
Our USB stack has a way to attach unconfigured USB devices to drivers,
meaning that such drivers have to call usbd_set_config_index() in their
*_attach() function. This is bad because *_attach() functions do not
return errors and, in the best case, the kernel ends up with an attached
but unusable driver.
In the worst case this ends up in a panic, generally when the device
is detached. I'll send one diff per panic as I found them with the
fuzzer I got. In the meantime I'd like to commit the diff below which
changes all the drivers using the first configuration, with index 0, to
let the stack call usbd_set_config_index() for them.
Ok?
Index: uhub.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.88
diff -u -p -r1.88 uhub.c
--- uhub.c 29 Nov 2015 16:30:48 -0000 1.88
+++ uhub.c 1 Sep 2016 12:47:31 -0000
@@ -102,11 +102,14 @@ uhub_match(struct device *parent, void *
struct usb_attach_arg *uaa = aux;
usb_device_descriptor_t *dd = usbd_get_device_descriptor(uaa->device);
+ if (uaa->iface == NULL)
+ return (UMATCH_NONE);
+
/*
* The subclass for hubs seems to be 0 for some and 1 for others,
* so we just ignore the subclass.
*/
- if (uaa->iface == NULL && dd->bDeviceClass == UDCLASS_HUB)
+ if (dd->bDeviceClass == UDCLASS_HUB)
return (UMATCH_DEVCLASS_DEVSUBCLASS);
return (UMATCH_NONE);
}
@@ -134,13 +137,6 @@ uhub_attach(struct device *parent, struc
sc->sc_hub = dev;
- err = usbd_set_config_index(dev, 0, 1);
- if (err) {
- DPRINTF("%s: configuration failed, error=%s\n",
- sc->sc_dev.dv_xname, usbd_errstr(err));
- return;
- }
-
if (dev->depth > USB_HUB_MAX_DEPTH) {
printf("%s: hub depth (%d) exceeded, hub ignored\n",
sc->sc_dev.dv_xname, USB_HUB_MAX_DEPTH);
@@ -324,10 +320,10 @@ uhub_attach(struct device *parent, struc
for (port = 1; port <= nports; port++) {
/* Turn the power on. */
err = usbd_set_port_feature(dev, port, UHF_PORT_POWER);
- if (err)
- printf("%s: port %d power on failed, %s\n",
- sc->sc_dev.dv_xname, port,
- usbd_errstr(err));
+ if (err) {
+ DPRINTF("%s: port %d power on failed, %s\n",
+ sc->sc_dev.dv_xname, port, usbd_errstr(err));
+ }
/* Make sure we check the port status at least once. */
sc->sc_status |= (1 << port);
}
Index: moscom.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/moscom.c,v
retrieving revision 1.23
diff -u -p -r1.23 moscom.c
--- moscom.c 7 Jan 2016 12:53:37 -0000 1.23
+++ moscom.c 1 Sep 2016 11:49:33 -0000
@@ -30,7 +30,6 @@
#include <dev/usb/ucomvar.h>
#define MOSCOMBUFSZ 256
-#define MOSCOM_CONFIG_INDEX 0
#define MOSCOM_IFACE_NO 0
#define MOSCOM_READ 0x0d
@@ -178,7 +177,7 @@ moscom_match(struct device *parent, void
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return UMATCH_NONE;
return (usb_lookup(moscom_devs, uaa->vendor, uaa->product) != NULL) ?
@@ -198,13 +197,6 @@ moscom_attach(struct device *parent, str
bzero(&uca, sizeof(uca));
sc->sc_udev = uaa->device;
-
- if (usbd_set_config_index(sc->sc_udev, MOSCOM_CONFIG_INDEX, 1) != 0) {
- printf("%s: could not set configuration no\n",
- sc->sc_dev.dv_xname);
- usbd_deactivate(sc->sc_udev);
- return;
- }
/* get the first interface handle */
error = usbd_device2interface_handle(sc->sc_udev, MOSCOM_IFACE_NO,
Index: uark.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uark.c,v
retrieving revision 1.23
diff -u -p -r1.23 uark.c
--- uark.c 7 Jan 2016 12:53:37 -0000 1.23
+++ uark.c 1 Sep 2016 11:50:05 -0000
@@ -38,7 +38,6 @@ int uarkebug = 0;
#define DPRINTF(x) DPRINTFN(0, x)
#define UARKBUFSZ 256
-#define UARK_CONFIG_INDEX 0
#define UARK_IFACE_NO 0
#define UARK_SET_DATA_BITS(x) (x - 5)
@@ -105,7 +104,7 @@ uark_match(struct device *parent, void *
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return UMATCH_NONE;
return (usb_lookup(uark_devs, uaa->vendor, uaa->product) != NULL) ?
@@ -125,13 +124,6 @@ uark_attach(struct device *parent, struc
bzero(&uca, sizeof(uca));
sc->sc_udev = uaa->device;
-
- if (usbd_set_config_index(sc->sc_udev, UARK_CONFIG_INDEX, 1) != 0) {
- printf("%s: could not set configuration no\n",
- sc->sc_dev.dv_xname);
- usbd_deactivate(sc->sc_udev);
- return;
- }
/* get the first interface handle */
error = usbd_device2interface_handle(sc->sc_udev, UARK_IFACE_NO,
Index: uchcom.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uchcom.c,v
retrieving revision 1.24
diff -u -p -r1.24 uchcom.c
--- uchcom.c 14 Apr 2015 07:57:33 -0000 1.24
+++ uchcom.c 1 Sep 2016 11:50:58 -0000
@@ -56,7 +56,6 @@ int uchcomdebug = 0;
#define DPRINTF(x) DPRINTFN(0, x)
#define UCHCOM_IFACE_INDEX 0
-#define UCHCOM_CONFIG_INDEX 0
#define UCHCOM_REV_CH340 0x0250
#define UCHCOM_INPUT_BUF_SIZE 8
@@ -165,7 +164,6 @@ int uchcom_open(void *, int);
void uchcom_close(void *, int);
void uchcom_intr(struct usbd_xfer *, void *, usbd_status);
-int uchcom_set_config(struct uchcom_softc *);
int uchcom_find_ifaces(struct uchcom_softc *,
struct usbd_interface **);
int uchcom_find_endpoints(struct uchcom_softc *,
@@ -237,7 +235,7 @@ uchcom_match(struct device *parent, void
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return UMATCH_NONE;
return (usb_lookup(uchcom_devs, uaa->vendor, uaa->product) != NULL ?
@@ -259,9 +257,6 @@ uchcom_attach(struct device *parent, str
DPRINTF(("\n\nuchcom attach: sc=%p\n", sc));
- if (uchcom_set_config(sc))
- goto failed;
-
switch (uaa->release) {
case UCHCOM_REV_CH340:
printf("%s: CH340\n", sc->sc_dev.dv_xname);
@@ -318,21 +313,6 @@ uchcom_detach(struct device *self, int f
}
return rv;
-}
-
-int
-uchcom_set_config(struct uchcom_softc *sc)
-{
- usbd_status err;
-
- err = usbd_set_config_index(sc->sc_udev, UCHCOM_CONFIG_INDEX, 1);
- if (err) {
- printf("%s: failed to set configuration: %s\n",
- sc->sc_dev.dv_xname, usbd_errstr(err));
- return -1;
- }
-
- return 0;
}
int
Index: udcf.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/udcf.c,v
retrieving revision 1.60
diff -u -p -r1.60 udcf.c
--- udcf.c 7 Jun 2015 20:11:52 -0000 1.60
+++ udcf.c 1 Sep 2016 11:51:35 -0000
@@ -147,7 +147,7 @@ udcf_match(struct device *parent, void *
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return UMATCH_NONE;
return (usb_lookup(udcf_devs, uaa->vendor, uaa->product) != NULL ?
@@ -205,12 +205,6 @@ udcf_attach(struct device *parent, struc
sensordev_install(&sc->sc_sensordev);
sc->sc_udev = dev;
- if ((err = usbd_set_config_index(dev, 0, 1))) {
- DPRINTF(("%s: failed to set configuration, err=%s\n",
- sc->sc_dev.dv_xname, usbd_errstr(err)));
- goto fishy;
- }
-
if ((err = usbd_device2interface_handle(dev, 0, &iface))) {
DPRINTF(("%s: failed to get interface, err=%s\n",
sc->sc_dev.dv_xname, usbd_errstr(err)));
Index: umbg.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/umbg.c,v
retrieving revision 1.23
diff -u -p -r1.23 umbg.c
--- umbg.c 12 Jul 2014 20:26:33 -0000 1.23
+++ umbg.c 1 Sep 2016 11:51:53 -0000
@@ -154,7 +154,7 @@ umbg_match(struct device *parent, void *
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return UMATCH_NONE;
return uaa->vendor == USB_VENDOR_MEINBERG &&
@@ -195,12 +195,6 @@ umbg_attach(struct device *parent, struc
usb_init_task(&sc->sc_task, umbg_task, sc, USB_TASK_TYPE_GENERIC);
timeout_set(&sc->sc_to, umbg_intr, sc);
timeout_set(&sc->sc_it_to, umbg_it_intr, sc);
-
- if ((err = usbd_set_config_index(dev, 0, 1))) {
- printf("%s: failed to set configuration, err=%s\n",
- sc->sc_dev.dv_xname, usbd_errstr(err));
- goto fishy;
- }
if ((err = usbd_device2interface_handle(dev, 0, &iface))) {
printf("%s: failed to get interface, err=%s\n",
Index: umct.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/umct.c,v
retrieving revision 1.44
diff -u -p -r1.44 umct.c
--- umct.c 26 Apr 2015 06:38:04 -0000 1.44
+++ umct.c 1 Sep 2016 11:52:19 -0000
@@ -66,7 +66,6 @@ int umctdebug = 0;
#endif
#define DPRINTF(x) DPRINTFN(0, x)
-#define UMCT_CONFIG_INDEX 0
#define UMCT_IFACE_INDEX 0
struct umct_softc {
@@ -159,7 +158,7 @@ umct_match(struct device *parent, void *
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return (UMATCH_NONE);
return (usb_lookup(umct_devs, uaa->vendor, uaa->product) != NULL ?
@@ -190,15 +189,6 @@ umct_attach(struct device *parent, struc
uca.bulkin = uca.bulkout = -1;
sc->sc_intr_number = -1;
sc->sc_intr_pipe = NULL;
-
- /* Move the device into the configured state. */
- err = usbd_set_config_index(dev, UMCT_CONFIG_INDEX, 1);
- if (err) {
- printf("\n%s: failed to set configuration, err=%s\n",
- devname, usbd_errstr(err));
- usbd_deactivate(sc->sc_udev);
- return;
- }
/* get the config descriptor */
cdesc = usbd_get_config_descriptor(sc->sc_udev);
Index: uonerng.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uonerng.c,v
retrieving revision 1.1
diff -u -p -r1.1 uonerng.c
--- uonerng.c 8 Jan 2016 09:36:59 -0000 1.1
+++ uonerng.c 1 Sep 2016 11:52:53 -0000
@@ -97,7 +97,6 @@
#define ONERNG_RF "cmd7\n"
-#define ONERNG_CONFIG_INDEX 0
#define ONERNG_IFACE_CTRL_INDEX 0
#define ONERNG_IFACE_DATA_INDEX 1
@@ -150,7 +149,7 @@ uonerng_match(struct device *parent, voi
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return UMATCH_NONE;
if (uaa->vendor != USB_VENDOR_OPENMOKO2 ||
@@ -178,13 +177,6 @@ uonerng_attach(struct device *parent, st
sc->sc_first_run = 1;
usb_init_task(&sc->sc_task, uonerng_task, sc, USB_TASK_TYPE_GENERIC);
-
- err = usbd_set_config_index(sc->sc_udev, ONERNG_CONFIG_INDEX, 1);
- if (err) {
- printf("%s: failed to set configuration, err=%s\n",
- DEVNAME(sc), usbd_errstr(err));
- goto fail;
- }
/* locate the control interface number and the data interface */
err = usbd_device2interface_handle(sc->sc_udev,
Index: uplcom.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uplcom.c,v
retrieving revision 1.67
diff -u -p -r1.67 uplcom.c
--- uplcom.c 24 May 2016 05:35:01 -0000 1.67
+++ uplcom.c 1 Sep 2016 11:53:11 -0000
@@ -66,7 +66,6 @@ int uplcomdebug = 0;
#endif
#define DPRINTF(x) DPRINTFN(0, x)
-#define UPLCOM_CONFIG_INDEX 0
#define UPLCOM_IFACE_INDEX 0
#define UPLCOM_SECOND_IFACE_INDEX 1
@@ -195,7 +194,7 @@ uplcom_match(struct device *parent, void
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return (UMATCH_NONE);
return (uplcom_lookup(uaa->vendor, uaa->product) != NULL ?
@@ -225,15 +224,6 @@ uplcom_attach(struct device *parent, str
uca.bulkin = uca.bulkout = -1;
sc->sc_intr_number = -1;
sc->sc_intr_pipe = NULL;
-
- /* Move the device into the configured state. */
- err = usbd_set_config_index(dev, UPLCOM_CONFIG_INDEX, 1);
- if (err) {
- printf("%s: failed to set configuration, err=%s\n",
- devname, usbd_errstr(err));
- usbd_deactivate(sc->sc_udev);
- return;
- }
/* get the config descriptor */
cdesc = usbd_get_config_descriptor(sc->sc_udev);
Index: uscom.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uscom.c,v
retrieving revision 1.4
diff -u -p -r1.4 uscom.c
--- uscom.c 7 Jan 2016 12:53:37 -0000 1.4
+++ uscom.c 1 Sep 2016 11:53:37 -0000
@@ -31,7 +31,6 @@
#include <dev/usb/ucomvar.h>
#define USCOMBUFSZ 256
-#define USCOM_CONFIG_INDEX 0
#define USCOM_IFACE_NO 0
struct uscom_softc {
@@ -73,7 +72,7 @@ uscom_match(struct device *parent, void
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return UMATCH_NONE;
return (usb_lookup(uscom_devs, uaa->vendor, uaa->product) != NULL) ?
@@ -93,13 +92,6 @@ uscom_attach(struct device *parent, stru
bzero(&uca, sizeof(uca));
sc->sc_udev = uaa->device;
-
- if (usbd_set_config_index(sc->sc_udev, USCOM_CONFIG_INDEX, 1) != 0) {
- printf("%s: could not set configuration no\n",
- sc->sc_dev.dv_xname);
- usbd_deactivate(sc->sc_udev);
- return;
- }
/* get the first interface handle */
error = usbd_device2interface_handle(sc->sc_udev, USCOM_IFACE_NO,
Index: uslcom.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uslcom.c,v
retrieving revision 1.38
diff -u -p -r1.38 uslcom.c
--- uslcom.c 31 Mar 2016 12:41:46 -0000 1.38
+++ uslcom.c 1 Sep 2016 11:54:01 -0000
@@ -38,7 +38,6 @@ int uslcomdebug = 0;
#define DPRINTF(x) DPRINTFN(0, x)
#define USLCOMBUFSZ 256
-#define USLCOM_CONFIG_INDEX 0
#define USLCOM_IFACE_NO 0
#define USLCOM_SET_DATA_BITS(x) (x << 8)
@@ -277,7 +276,7 @@ uslcom_match(struct device *parent, void
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return UMATCH_NONE;
return (usb_lookup(uslcom_devs, uaa->vendor, uaa->product) != NULL) ?
@@ -297,13 +296,6 @@ uslcom_attach(struct device *parent, str
bzero(&uca, sizeof(uca));
sc->sc_udev = uaa->device;
-
- if (usbd_set_config_index(sc->sc_udev, USLCOM_CONFIG_INDEX, 1) != 0) {
- printf("%s: could not set configuration no\n",
- sc->sc_dev.dv_xname);
- usbd_deactivate(sc->sc_udev);
- return;
- }
/* get the first interface handle */
error = usbd_device2interface_handle(sc->sc_udev, USLCOM_IFACE_NO,
Index: uts.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uts.c,v
retrieving revision 1.38
diff -u -p -r1.38 uts.c
--- uts.c 5 Jun 2016 20:15:54 -0000 1.38
+++ uts.c 1 Sep 2016 11:54:37 -0000
@@ -44,8 +44,6 @@
#define DPRINTF(x)
#endif
-#define UTS_CONFIG_INDEX 0
-
struct tsscale {
int minx, maxx;
int miny, maxy;
@@ -162,14 +160,6 @@ uts_attach(struct device *parent, struct
/* Copy the default scalue values to each softc */
bcopy(&def_scale, &sc->sc_tsscale, sizeof(sc->sc_tsscale));
-
- /* Move the device into the configured state. */
- if (usbd_set_config_index(uaa->device, UTS_CONFIG_INDEX, 1) != 0) {
- printf("%s: could not set configuartion no\n",
- sc->sc_dev.dv_xname);
- usbd_deactivate(sc->sc_udev);
- return;
- }
/* get the config descriptor */
cdesc = usbd_get_config_descriptor(sc->sc_udev);
Index: uvisor.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uvisor.c,v
retrieving revision 1.50
diff -u -p -r1.50 uvisor.c
--- uvisor.c 14 Mar 2015 03:38:50 -0000 1.50
+++ uvisor.c 1 Sep 2016 11:54:57 -0000
@@ -59,7 +59,6 @@ int uvisordebug = 0;
#define DPRINTFN(n,x)
#endif
-#define UVISOR_CONFIG_INDEX 0
#define UVISOR_IFACE_INDEX 0
/* From the Linux driver */
@@ -209,7 +208,7 @@ uvisor_match(struct device *parent, void
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return (UMATCH_NONE);
DPRINTFN(20,("uvisor: vendor=0x%x, product=0x%x\n",
@@ -235,14 +234,6 @@ uvisor_attach(struct device *parent, str
struct ucom_attach_args uca;
DPRINTFN(10,("\nuvisor_attach: sc=%p\n", sc));
-
- /* Move the device into the configured state. */
- err = usbd_set_config_index(dev, UVISOR_CONFIG_INDEX, 1);
- if (err) {
- printf(": failed to set configuration, err=%s\n",
- usbd_errstr(err));
- goto bad;
- }
err = usbd_device2interface_handle(dev, UVISOR_IFACE_INDEX, &iface);
if (err) {
Index: uvscom.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uvscom.c,v
retrieving revision 1.33
diff -u -p -r1.33 uvscom.c
--- uvscom.c 14 Mar 2015 03:38:50 -0000 1.33
+++ uvscom.c 1 Sep 2016 11:55:19 -0000
@@ -65,7 +65,6 @@ static int uvscomdebug = 1;
#endif
#define DPRINTF(x) DPRINTFN(0, x)
-#define UVSCOM_CONFIG_INDEX 0
#define UVSCOM_IFACE_INDEX 0
#define UVSCOM_INTR_INTERVAL 100 /* mS */
@@ -219,7 +218,7 @@ uvscom_match(struct device *parent, void
{
struct usb_attach_arg *uaa = aux;
- if (uaa->iface != NULL)
+ if (uaa->iface == NULL)
return (UMATCH_NONE);
return (usb_lookup(uvscom_devs, uaa->vendor, uaa->product) != NULL ?
@@ -248,15 +247,6 @@ uvscom_attach(struct device *parent, str
uca.bulkin = uca.bulkout = -1;
sc->sc_intr_number = -1;
sc->sc_intr_pipe = NULL;
-
- /* Move the device into the configured state. */
- err = usbd_set_config_index(dev, UVSCOM_CONFIG_INDEX, 1);
- if (err) {
- printf("%s: failed to set configuration, err=%s\n",
- devname, usbd_errstr(err));
- usbd_deactivate(sc->sc_udev);
- return;
- }
/* get the config descriptor */
cdesc = usbd_get_config_descriptor(sc->sc_udev);
