Skip to content

Commit 47d4aac

Browse files
Pull up following revision(s) (requested by riastradh in ticket #58):
sys/dev/usb/xhci.c: revision 1.191 sys/dev/usb/usb_subr.c: revision 1.280 sys/dev/usb/usb_subr.c: revision 1.281 sys/dev/usb/usbdivar.h: revision 1.140 usb(9): Record config index, not just number, in struct usbd_device. The index is a zero-based index in [0, bNumConfigurations), or -1 for unconfigured. The number is an arbitrary value of a config descriptor's bConfigurationValue field, or 0 for unconfigured -- with the tricky caveat that bConfigurationValue might also be 0. Preparation for fixing: PR kern/59185: panic over KASSERTMSG(dev->ud_ifaces == NULL) on Dell Latitude 7490 PR kern/59624: Booting NetBSD-11 from USB on my Dell machine panics and hangs PR kern/57447: HEAD fails to probe USB devices and fails to boot up usb(9): Use ud_configidx, not ud_config, to see if unconfigured. ud_config is a device-provided quantity in the config descriptor's bConfigurationValue, and a faulty (or malicious) device can provide 0 for that value, which coincides with our software sentinel value USBD_UNCONFIG_NO of 0. Instead of testing ud_config, test ud_configidx, which is an index in [0, bNumConfigurations) or -1, for which the device cannot confuse us by a value that coincides with the sentinel -1. PR kern/59185: panic over KASSERTMSG(dev->ud_ifaces == NULL) on Dell Latitude 7490 PR kern/59624: Booting NetBSD-11 from USB on my Dell machine panics and hangs PR kern/57447: HEAD fails to probe USB devices and fails to boot up
1 parent 62e44e4 commit 47d4aac

File tree

3 files changed

+36
-11
lines changed

3 files changed

+36
-11
lines changed

sys/dev/usb/usb_subr.c

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: usb_subr.c,v 1.279 2024/05/04 12:45:13 mlelstv Exp $ */
1+
/* $NetBSD: usb_subr.c,v 1.279.4.1 2025/10/19 10:08:32 martin Exp $ */
22
/* $FreeBSD: src/sys/dev/usb/usb_subr.c,v 1.18 1999/11/17 22:33:47 n_hibma Exp $ */
33

44
/*
@@ -32,7 +32,7 @@
3232
*/
3333

3434
#include <sys/cdefs.h>
35-
__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.279 2024/05/04 12:45:13 mlelstv Exp $");
35+
__KERNEL_RCSID(0, "$NetBSD: usb_subr.c,v 1.279.4.1 2025/10/19 10:08:32 martin Exp $");
3636

3737
#ifdef _KERNEL_OPT
3838
#include "opt_compat_netbsd.h"
@@ -154,7 +154,6 @@ usbd_get_device_strings(struct usbd_device *ud)
154154
usbd_get_device_string(ud, udd->iSerialNumber, &ud->ud_serial);
155155
}
156156

157-
158157
void
159158
usbd_devinfo_vp(struct usbd_device *dev, char *v, size_t vl, char *p,
160159
size_t pl, int usedev, int useencoded)
@@ -691,16 +690,15 @@ usbd_set_config_index(struct usbd_device *dev, int index, int msg)
691690
usbd_status err;
692691
int i, ifcidx, nifc, len, selfpowered, power;
693692

694-
695-
if (index >= dev->ud_ddesc.bNumConfigurations &&
693+
if ((unsigned)index >= dev->ud_ddesc.bNumConfigurations &&
696694
index != USB_UNCONFIG_INDEX) {
697695
/* panic? */
698696
printf("usbd_set_config_index: illegal index\n");
699697
return USBD_INVAL;
700698
}
701699

702700
/* XXX check that all interfaces are idle */
703-
if (dev->ud_config != USB_UNCONFIG_NO) {
701+
if (dev->ud_configidx != USB_UNCONFIG_INDEX) {
704702
DPRINTF("free old config", 0, 0, 0, 0);
705703
/* Free all configuration data structures. */
706704
nifc = dev->ud_cdesc->bNumInterface;
@@ -719,7 +717,12 @@ usbd_set_config_index(struct usbd_device *dev, int index, int msg)
719717
dev->ud_cdesc = NULL;
720718
dev->ud_bdesc = NULL;
721719
dev->ud_config = USB_UNCONFIG_NO;
720+
dev->ud_configidx = USB_UNCONFIG_INDEX;
722721
}
722+
KASSERTMSG(dev->ud_config == USB_UNCONFIG_NO, "ud_config=%u",
723+
dev->ud_config);
724+
KASSERTMSG(dev->ud_configidx == USB_UNCONFIG_INDEX, "ud_configidx=%d",
725+
dev->ud_configidx);
723726

724727
if (index == USB_UNCONFIG_INDEX) {
725728
/* We are unconfiguring the device, so leave unallocated. */
@@ -882,6 +885,7 @@ usbd_set_config_index(struct usbd_device *dev, int index, int msg)
882885
0, 0);
883886
dev->ud_cdesc = cdp;
884887
dev->ud_config = cdp->bConfigurationValue;
888+
dev->ud_configidx = index;
885889
for (ifcidx = 0; ifcidx < nifc; ifcidx++) {
886890
usbd_iface_init(dev, ifcidx);
887891
usbd_iface_exlock(&dev->ud_ifaces[ifcidx]);
@@ -905,8 +909,8 @@ usbd_set_config_index(struct usbd_device *dev, int index, int msg)
905909

906910
bad:
907911
/* XXX Use usbd_set_config() to reset the config? */
908-
/* XXX Should we forbid USB_UNCONFIG_NO from bConfigurationValue? */
909912
dev->ud_config = USB_UNCONFIG_NO;
913+
dev->ud_configidx = USB_UNCONFIG_INDEX;
910914
KASSERT(dev->ud_ifaces == NULL);
911915
kmem_free(cdp, len);
912916
dev->ud_cdesc = NULL;
@@ -1194,7 +1198,6 @@ usbd_attachinterfaces(device_t parent, struct usbd_device *dev,
11941198
DPRINTF("interface %jd %#jx", i, (uintptr_t)ifaces[i], 0, 0);
11951199
}
11961200

1197-
11981201
uiaa.uiaa_device = dev;
11991202
uiaa.uiaa_port = port;
12001203
uiaa.uiaa_vendor = UGETW(dd->idVendor);
@@ -1446,6 +1449,8 @@ usbd_new_device(device_t parent, struct usbd_bus *bus, int depth, int speed,
14461449
dev->ud_quirks = &usbd_no_quirk;
14471450
dev->ud_addr = USB_START_ADDR;
14481451
dev->ud_ddesc.bMaxPacketSize = 0;
1452+
dev->ud_config = USB_UNCONFIG_NO;
1453+
dev->ud_configidx = USB_UNCONFIG_INDEX;
14491454
dev->ud_depth = depth;
14501455
dev->ud_powersrc = up;
14511456
dev->ud_myhub = up->up_parent;

sys/dev/usb/usbdivar.h

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: usbdivar.h,v 1.139 2025/03/31 14:45:14 riastradh Exp $ */
1+
/* $NetBSD: usbdivar.h,v 1.139.2.1 2025/10/19 10:08:32 martin Exp $ */
22

33
/*
44
* Copyright (c) 1998, 2012 The NetBSD Foundation, Inc.
@@ -230,6 +230,24 @@ struct usbd_device {
230230
char *ud_serial; /* serial number, can be NULL */
231231
char *ud_vendor; /* vendor string, can be NULL */
232232
char *ud_product; /* product string can be NULL */
233+
234+
/*
235+
* ud_config above holds a value of bConfigurationValue from
236+
* the config descriptor, or USB_UNCONFIG_NO=0 -- which may
237+
* _also_ be a value of bConfigurationValue.
238+
*
239+
* ud_configidx below holds an index in [0, bNumConfigurations)
240+
* into the list of configuration descriptors, or
241+
* USB_UNCONFIG_INDEX=-1 to denote that the interface is
242+
* unconfigured. Note that ud_config may be USB_UNCONFIG_NO
243+
* even if ud_configidx is not USB_UNCONFIG_INDEX, if a screwy
244+
* device has a config descriptor with bConfigurationValue=0.
245+
*
246+
* This goes at the end, rather than next to ud_config where it
247+
* might properly belong, so the change preserves ABI for
248+
* pullup to release branches.
249+
*/
250+
int16_t ud_configidx;
233251
};
234252

235253
struct usbd_interface {

sys/dev/usb/xhci.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $NetBSD: xhci.c,v 1.188.2.2 2025/10/11 10:43:25 martin Exp $ */
1+
/* $NetBSD: xhci.c,v 1.188.2.3 2025/10/19 10:08:32 martin Exp $ */
22

33
/*
44
* Copyright (c) 2013 Jonathan A. Kollasch
@@ -34,7 +34,7 @@
3434
*/
3535

3636
#include <sys/cdefs.h>
37-
__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.188.2.2 2025/10/11 10:43:25 martin Exp $");
37+
__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.188.2.3 2025/10/19 10:08:32 martin Exp $");
3838

3939
#ifdef _KERNEL_OPT
4040
#include "opt_usb.h"
@@ -2860,6 +2860,8 @@ xhci_new_device(device_t parent, struct usbd_bus *bus, int depth,
28602860
dev->ud_quirks = &usbd_no_quirk;
28612861
dev->ud_addr = 0;
28622862
dev->ud_ddesc.bMaxPacketSize = 0;
2863+
dev->ud_config = USB_UNCONFIG_NO;
2864+
dev->ud_configidx = USB_UNCONFIG_INDEX;
28632865
dev->ud_depth = depth;
28642866
dev->ud_powersrc = up;
28652867
dev->ud_myhub = up->up_parent;

0 commit comments

Comments
 (0)