Re: kern/59185: panic over KASSERTMSG(dev->ud_ifaces == NULL) on Dell Latitude 7490

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

> Date: Sat, 4 Oct 2025 19:06:11 +0000
> From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
> 
> > Date: Sat, 4 Oct 2025 19:03:59 +0000
> > From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
> > 
> > Can you please try the attached patch and see if it helps?
> > 
> > I suspect this is the same issue as:
> > 
> > PR kern/59624: Booting NetBSD-11 from USB on my Dell machine panics
> > and hangs
> > https://gnats.NetBSD.org/59624
> > 
> > PR kern/57447: HEAD fails to probe USB devices and fails to boot up
> > https://gnats.NetBSD.org/57447
> > 
> > syzbot: UBSan: Undefined Behavior in usb_free_device (2)
> > https://syzkaller.appspot.com/bug?id=e6d4449a128e73a9a88100a5cc833e5cae9fecae
> 
> ...patch attached this time

Corrected patch attached!

diff -r 1c25535fd2c2 sys/compat/common/usb_subr_30.c
--- a/sys/compat/common/usb_subr_30.c	Mon Sep 29 17:01:48 2025 +0000
+++ b/sys/compat/common/usb_subr_30.c	Sat Oct 04 19:41:08 2025 +0000
@@ -147,7 +147,7 @@ usbd_fill_deviceinfo30(struct usbd_devic
 	di->udi_class = dev->ud_ddesc.bDeviceClass;
 	di->udi_subclass = dev->ud_ddesc.bDeviceSubClass;
 	di->udi_protocol = dev->ud_ddesc.bDeviceProtocol;
-	di->udi_config = dev->ud_config;
+	di->udi_config = dev->ud_configno;
 	di->udi_power = dev->ud_selfpowered ? 0 : dev->ud_power;
 	di->udi_speed = dev->ud_speed;
 
diff -r 1c25535fd2c2 sys/dev/usb/usb_subr.c
--- a/sys/dev/usb/usb_subr.c	Mon Sep 29 17:01:48 2025 +0000
+++ b/sys/dev/usb/usb_subr.c	Sat Oct 04 19:41:08 2025 +0000
@@ -154,7 +154,6 @@ usbd_get_device_strings(struct usbd_devi
 	usbd_get_device_string(ud, udd->iSerialNumber, &ud->ud_serial);
 }
 
-
 void
 usbd_devinfo_vp(struct usbd_device *dev, char *v, size_t vl, char *p,
     size_t pl, int usedev, int useencoded)
@@ -691,8 +690,7 @@ usbd_set_config_index(struct usbd_device
 	usbd_status err;
 	int i, ifcidx, nifc, len, selfpowered, power;
 
-
-	if (index >= dev->ud_ddesc.bNumConfigurations &&
+	if ((unsigned)index >= dev->ud_ddesc.bNumConfigurations &&
 	    index != USB_UNCONFIG_INDEX) {
 		/* panic? */
 		printf("usbd_set_config_index: illegal index\n");
@@ -700,7 +698,7 @@ usbd_set_config_index(struct usbd_device
 	}
 
 	/* XXX check that all interfaces are idle */
-	if (dev->ud_config != USB_UNCONFIG_NO) {
+	if (dev->ud_configidx != USB_UNCONFIG_INDEX) {
 		DPRINTF("free old config", 0, 0, 0, 0);
 		/* Free all configuration data structures. */
 		nifc = dev->ud_cdesc->bNumInterface;
@@ -718,8 +716,13 @@ usbd_set_config_index(struct usbd_device
 		dev->ud_ifaces = NULL;
 		dev->ud_cdesc = NULL;
 		dev->ud_bdesc = NULL;
-		dev->ud_config = USB_UNCONFIG_NO;
+		dev->ud_configno = USB_UNCONFIG_NO;
+		dev->ud_configidx = USB_UNCONFIG_INDEX;
 	}
+	KASSERTMSG(dev->ud_configno == USB_UNCONFIG_NO, "ud_configno=%u",
+	    dev->ud_configno);
+	KASSERTMSG(dev->ud_configidx == USB_UNCONFIG_INDEX, "ud_configidx=%d",
+	    dev->ud_configidx);
 
 	if (index == USB_UNCONFIG_INDEX) {
 		/* We are unconfiguring the device, so leave unallocated. */
@@ -881,7 +884,8 @@ usbd_set_config_index(struct usbd_device
 	DPRINTFN(5, "dev=%#jx cdesc=%#jx", (uintptr_t)dev, (uintptr_t)cdp,
 	    0, 0);
 	dev->ud_cdesc = cdp;
-	dev->ud_config = cdp->bConfigurationValue;
+	dev->ud_configno = cdp->bConfigurationValue;
+	dev->ud_configidx = index;
 	for (ifcidx = 0; ifcidx < nifc; ifcidx++) {
 		usbd_iface_init(dev, ifcidx);
 		usbd_iface_exlock(&dev->ud_ifaces[ifcidx]);
@@ -905,8 +909,8 @@ usbd_set_config_index(struct usbd_device
 
 bad:
 	/* XXX Use usbd_set_config() to reset the config? */
-	/* XXX Should we forbid USB_UNCONFIG_NO from bConfigurationValue? */
-	dev->ud_config = USB_UNCONFIG_NO;
+	dev->ud_configno = USB_UNCONFIG_NO;
+	dev->ud_configidx = USB_UNCONFIG_INDEX;
 	KASSERT(dev->ud_ifaces == NULL);
 	kmem_free(cdp, len);
 	dev->ud_cdesc = NULL;
@@ -1194,7 +1198,6 @@ usbd_attachinterfaces(device_t parent, s
 		DPRINTF("interface %jd %#jx", i, (uintptr_t)ifaces[i], 0, 0);
 	}
 
-
 	uiaa.uiaa_device = dev;
 	uiaa.uiaa_port = port;
 	uiaa.uiaa_vendor = UGETW(dd->idVendor);
@@ -1446,6 +1449,8 @@ usbd_new_device(device_t parent, struct 
 	dev->ud_quirks = &usbd_no_quirk;
 	dev->ud_addr = USB_START_ADDR;
 	dev->ud_ddesc.bMaxPacketSize = 0;
+	dev->ud_configno = USB_UNCONFIG_NO;
+	dev->ud_configidx = USB_UNCONFIG_INDEX;
 	dev->ud_depth = depth;
 	dev->ud_powersrc = up;
 	dev->ud_myhub = up->up_parent;
@@ -1776,7 +1781,7 @@ usbd_fill_deviceinfo(struct usbd_device 
 	di->udi_class = dev->ud_ddesc.bDeviceClass;
 	di->udi_subclass = dev->ud_ddesc.bDeviceSubClass;
 	di->udi_protocol = dev->ud_ddesc.bDeviceProtocol;
-	di->udi_config = dev->ud_config;
+	di->udi_config = dev->ud_configno;
 	di->udi_power = dev->ud_selfpowered ? 0 : dev->ud_power;
 	di->udi_speed = dev->ud_speed;
 
diff -r 1c25535fd2c2 sys/dev/usb/usbdi.c
--- a/sys/dev/usb/usbdi.c	Mon Sep 29 17:01:48 2025 +0000
+++ b/sys/dev/usb/usbdi.c	Sat Oct 04 19:41:08 2025 +0000
@@ -169,7 +169,7 @@ usbd_dump_device(struct usbd_device *dev
 	USBHIST_LOG(usbdebug, "     bus = %#jx default_pipe = %#jx",
 	    (uintptr_t)dev->ud_bus, (uintptr_t)dev->ud_pipe0, 0, 0);
 	USBHIST_LOG(usbdebug, "     address = %jd config = %jd depth = %jd ",
-	    dev->ud_addr, dev->ud_config, dev->ud_depth, 0);
+	    dev->ud_addr, dev->ud_configno, dev->ud_depth, 0);
 	USBHIST_LOG(usbdebug, "     speed = %jd self_powered = %jd "
 	    "power = %jd langid = %jd",
 	    dev->ud_speed, dev->ud_selfpowered, dev->ud_power, dev->ud_langid);
diff -r 1c25535fd2c2 sys/dev/usb/usbdivar.h
--- a/sys/dev/usb/usbdivar.h	Mon Sep 29 17:01:48 2025 +0000
+++ b/sys/dev/usb/usbdivar.h	Sat Oct 04 19:41:08 2025 +0000
@@ -201,7 +201,7 @@ struct usbd_device {
 	struct usbd_bus	       *ud_bus;		/* our controller */
 	struct usbd_pipe       *ud_pipe0;	/* pipe 0 */
 	uint8_t			ud_addr;	/* device address */
-	uint8_t			ud_config;	/* current configuration # */
+	uint8_t			ud_configno;	/* current configuration # */
 	uint8_t			ud_depth;	/* distance from root hub */
 	uint8_t			ud_speed;	/* low/full/high speed */
 	uint8_t			ud_selfpowered;	/* flag for self powered */
@@ -230,6 +230,26 @@ struct usbd_device {
 	char		       *ud_serial;	/* serial number, can be NULL */
 	char		       *ud_vendor;	/* vendor string, can be NULL */
 	char		       *ud_product;	/* product string can be NULL */
+
+	/*
+	 * ud_configno above holds a value of bConfigurationValue from
+	 * the config descriptor, or USB_UNCONFIG_NO=0 -- which may
+	 * _also_ be a value of bConfigurationValue.
+	 *
+	 * ud_configidx below holds an index in [0, bNumConfigurations)
+	 * into the list of configuration descriptors, or
+	 * USB_UNCONFIG_INDEX=-1 to denote that the interface is
+	 * unconfigured.  Note that ud_configno may be USB_UNCONFIG_NO
+	 * even if ud_configidx is not USB_UNCONFIG_INDEX, if a screwy
+	 * device has a config descriptor with bConfigurationValue=0.
+	 *
+	 * This goes at the end, rather than next to ud_configno where
+	 * it might properly belong, so the change preserves ABI for
+	 * pullup to release branches.
+	 */
+	int16_t			ud_configidx;
+
+	uint8_t			ud_extra[];	/* prevent embedding */
 };
 
 struct usbd_interface {
diff -r 1c25535fd2c2 sys/dev/usb/xhci.c
--- a/sys/dev/usb/xhci.c	Mon Sep 29 17:01:48 2025 +0000
+++ b/sys/dev/usb/xhci.c	Sat Oct 04 19:41:08 2025 +0000
@@ -2861,6 +2861,8 @@ xhci_new_device(device_t parent, struct 
 	dev->ud_quirks = &usbd_no_quirk;
 	dev->ud_addr = 0;
 	dev->ud_ddesc.bMaxPacketSize = 0;
+	dev->ud_configno = USB_UNCONFIG_NO;
+	dev->ud_configidx = USB_UNCONFIG_INDEX;
 	dev->ud_depth = depth;
 	dev->ud_powersrc = up;
 	dev->ud_myhub = up->up_parent;