NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



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

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: "Emile `iMil' Heitor" <imil%home.imil.net@localhost>,
	Emmanuel Nyarko <emmankoko519%gmail.com@localhost>,
	Salil Wadnerkar <bsdprg%tuta.io@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Subject: Re: kern/59185: panic over KASSERTMSG(dev->ud_ifaces == NULL) on Dell Latitude 7490
Date: Sat, 4 Oct 2025 19:42:25 +0000

 This is a multi-part message in MIME format.
 --=_yEQegd6KE+J0RR5NTU8CY9iUdk+bjG6u
 Content-Transfer-Encoding: quoted-printable
 
 > Date: Sat, 4 Oct 2025 19:06:11 +0000
 > From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
 >=20
 > > Date: Sat, 4 Oct 2025 19:03:59 +0000
 > > From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
 > >=20
 > > Can you please try the attached patch and see if it helps?
 > >=20
 > > I suspect this is the same issue as:
 > >=20
 > > PR kern/59624: Booting NetBSD-11 from USB on my Dell machine panics
 > > and hangs
 > > https://gnats.NetBSD.org/59624
 > >=20
 > > PR kern/57447: HEAD fails to probe USB devices and fails to boot up
 > > https://gnats.NetBSD.org/57447
 > >=20
 > > syzbot: UBSan: Undefined Behavior in usb_free_device (2)
 > > https://syzkaller.appspot.com/bug?id=3De6d4449a128e73a9a88100a5cc833e5c=
 ae9fecae
 >=20
 > ...patch attached this time
 
 Corrected patch attached!
 
 --=_yEQegd6KE+J0RR5NTU8CY9iUdk+bjG6u
 Content-Type: text/plain; charset="ISO-8859-1"; name="pr59185-usbconfignoabuse-v2"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="pr59185-usbconfignoabuse-v2.diff"
 
 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 =3D dev->ud_ddesc.bDeviceClass;
  	di->udi_subclass =3D dev->ud_ddesc.bDeviceSubClass;
  	di->udi_protocol =3D dev->ud_ddesc.bDeviceProtocol;
 -	di->udi_config =3D dev->ud_config;
 +	di->udi_config =3D dev->ud_configno;
  	di->udi_power =3D dev->ud_selfpowered ? 0 : dev->ud_power;
  	di->udi_speed =3D dev->ud_speed;
 =20
 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);
  }
 =20
 -
  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;
 =20
 -
 -	if (index >=3D dev->ud_ddesc.bNumConfigurations &&
 +	if ((unsigned)index >=3D dev->ud_ddesc.bNumConfigurations &&
  	    index !=3D USB_UNCONFIG_INDEX) {
  		/* panic? */
  		printf("usbd_set_config_index: illegal index\n");
 @@ -700,7 +698,7 @@ usbd_set_config_index(struct usbd_device
  	}
 =20
  	/* XXX check that all interfaces are idle */
 -	if (dev->ud_config !=3D USB_UNCONFIG_NO) {
 +	if (dev->ud_configidx !=3D USB_UNCONFIG_INDEX) {
  		DPRINTF("free old config", 0, 0, 0, 0);
  		/* Free all configuration data structures. */
  		nifc =3D dev->ud_cdesc->bNumInterface;
 @@ -718,8 +716,13 @@ usbd_set_config_index(struct usbd_device
  		dev->ud_ifaces =3D NULL;
  		dev->ud_cdesc =3D NULL;
  		dev->ud_bdesc =3D NULL;
 -		dev->ud_config =3D USB_UNCONFIG_NO;
 +		dev->ud_configno =3D USB_UNCONFIG_NO;
 +		dev->ud_configidx =3D USB_UNCONFIG_INDEX;
  	}
 +	KASSERTMSG(dev->ud_configno =3D=3D USB_UNCONFIG_NO, "ud_configno=3D%u",
 +	    dev->ud_configno);
 +	KASSERTMSG(dev->ud_configidx =3D=3D USB_UNCONFIG_INDEX, "ud_configidx=3D%=
 d",
 +	    dev->ud_configidx);
 =20
  	if (index =3D=3D 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=3D%#jx cdesc=3D%#jx", (uintptr_t)dev, (uintptr_t)cdp,
  	    0, 0);
  	dev->ud_cdesc =3D cdp;
 -	dev->ud_config =3D cdp->bConfigurationValue;
 +	dev->ud_configno =3D cdp->bConfigurationValue;
 +	dev->ud_configidx =3D index;
  	for (ifcidx =3D 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
 =20
  bad:
  	/* XXX Use usbd_set_config() to reset the config? */
 -	/* XXX Should we forbid USB_UNCONFIG_NO from bConfigurationValue? */
 -	dev->ud_config =3D USB_UNCONFIG_NO;
 +	dev->ud_configno =3D USB_UNCONFIG_NO;
 +	dev->ud_configidx =3D USB_UNCONFIG_INDEX;
  	KASSERT(dev->ud_ifaces =3D=3D NULL);
  	kmem_free(cdp, len);
  	dev->ud_cdesc =3D NULL;
 @@ -1194,7 +1198,6 @@ usbd_attachinterfaces(device_t parent, s
  		DPRINTF("interface %jd %#jx", i, (uintptr_t)ifaces[i], 0, 0);
  	}
 =20
 -
  	uiaa.uiaa_device =3D dev;
  	uiaa.uiaa_port =3D port;
  	uiaa.uiaa_vendor =3D UGETW(dd->idVendor);
 @@ -1446,6 +1449,8 @@ usbd_new_device(device_t parent, struct=20
  	dev->ud_quirks =3D &usbd_no_quirk;
  	dev->ud_addr =3D USB_START_ADDR;
  	dev->ud_ddesc.bMaxPacketSize =3D 0;
 +	dev->ud_configno =3D USB_UNCONFIG_NO;
 +	dev->ud_configidx =3D USB_UNCONFIG_INDEX;
  	dev->ud_depth =3D depth;
  	dev->ud_powersrc =3D up;
  	dev->ud_myhub =3D up->up_parent;
 @@ -1776,7 +1781,7 @@ usbd_fill_deviceinfo(struct usbd_device=20
  	di->udi_class =3D dev->ud_ddesc.bDeviceClass;
  	di->udi_subclass =3D dev->ud_ddesc.bDeviceSubClass;
  	di->udi_protocol =3D dev->ud_ddesc.bDeviceProtocol;
 -	di->udi_config =3D dev->ud_config;
 +	di->udi_config =3D dev->ud_configno;
  	di->udi_power =3D dev->ud_selfpowered ? 0 : dev->ud_power;
  	di->udi_speed =3D dev->ud_speed;
 =20
 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 =3D %#jx default_pipe =3D %#jx",
  	    (uintptr_t)dev->ud_bus, (uintptr_t)dev->ud_pipe0, 0, 0);
  	USBHIST_LOG(usbdebug, "     address =3D %jd config =3D %jd depth =3D %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 =3D %jd self_powered =3D %jd "
  	    "power =3D %jd langid =3D %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=3D0 -- 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=3D-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=3D0.
 +	 *
 +	 * 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 */
  };
 =20
  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=20
  	dev->ud_quirks =3D &usbd_no_quirk;
  	dev->ud_addr =3D 0;
  	dev->ud_ddesc.bMaxPacketSize =3D 0;
 +	dev->ud_configno =3D USB_UNCONFIG_NO;
 +	dev->ud_configidx =3D USB_UNCONFIG_INDEX;
  	dev->ud_depth =3D depth;
  	dev->ud_powersrc =3D up;
  	dev->ud_myhub =3D up->up_parent;
 
 --=_yEQegd6KE+J0RR5NTU8CY9iUdk+bjG6u--
 


Home | Main Index | Thread Index | Old Index