From: "Theo de Raadt" <deraadt@openbsd.org>
Subject: Re: 
To: tech@openbsd.org, djm@openbsd.org
Date: Mon, 07 Sep 2026 12:50:45 -0600

Theo de Raadt <deraadt@openbsd.org> wrote:

> Kirill A. Korinsky <kirill@korins.ky> wrote:
> 
> > my update libfido accidently reverted a hardcoded USB device metadata which
> > leads broken pledge in, for example, chromium.
> 
> I'm more willing to allow pledge to reach ioctl USB_GET_DEVICEINFO, if
> there are some incremental safety improvements made to the kernel code
> that is reached.
> 
> I think usbd_fill_deviceinfo() should return int, look carefully at
> what it is going to provide to userland, and return an error if the
> usb configuration is not sane.
> 
> But first, I think we should purge old data in the structure.
> 
> Index: usb_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
> diff -u -p -u -r1.167 usb_subr.c
> --- usb_subr.c	15 Aug 2026 22:06:40 -0000	1.167
> +++ usb_subr.c	7 Sep 2026 18:36:54 -0000
> @@ -1298,6 +1298,7 @@ usbd_fill_deviceinfo(struct usbd_device 
>  	struct usbd_port *p;
>  	int i;
>  
> +	memset(di, 0, sizeof(*di));
>  	di->udi_bus = dev->bus->usbctl->dv_unit;
>  	di->udi_addr = dev->address;
>  	strlcpy(di->udi_vendor, dev->vendor, sizeof(di->udi_vendor));

If the structure is zero'd, then usbd_fill_deviceinfo() only needs
to focus on filling in non-zero values, which looks like this:

Index: usb_subr.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
diff -u -p -u -r1.167 usb_subr.c
--- usb_subr.c	15 Aug 2026 22:06:40 -0000	1.167
+++ usb_subr.c	7 Sep 2026 18:48:37 -0000
@@ -1298,6 +1298,7 @@ usbd_fill_deviceinfo(struct usbd_device 
 	struct usbd_port *p;
 	int i;
 
+	memset(di, 0, sizeof(*di));
 	di->udi_bus = dev->bus->usbctl->dv_unit;
 	di->udi_addr = dev->address;
 	strlcpy(di->udi_vendor, dev->vendor, sizeof(di->udi_vendor));
@@ -1316,16 +1317,10 @@ usbd_fill_deviceinfo(struct usbd_device 
 	di->udi_port = dev->powersrc ? dev->powersrc->portno : 0;
 
 	if (dev->subdevs != NULL) {
-		for (i = 0; dev->subdevs[i] && i < USB_MAX_DEVNAMES; i++) {
-			strncpy(di->udi_devnames[i],
+		for (i = 0; dev->subdevs[i] && i < USB_MAX_DEVNAMES; i++)
+			strlcpy(di->udi_devnames[i],
 			    dev->subdevs[i]->dv_xname, USB_MAX_DEVNAMELEN);
-			di->udi_devnames[i][USB_MAX_DEVNAMELEN-1] = '\0';
-		}
-	} else
-		i = 0;
-
-	for (/*i is set */; i < USB_MAX_DEVNAMES; i++)
-		di->udi_devnames[i][0] = 0; /* empty */
+	}
 
 	if (dev->hub) {
 		for (i = 0;
@@ -1335,13 +1330,10 @@ usbd_fill_deviceinfo(struct usbd_device 
 			    UGETW(p->status.wPortStatus);
 		}
 		di->udi_nports = dev->hub->nports;
-	} else
-		di->udi_nports = 0;
+	}
 
-	bzero(di->udi_serial, sizeof(di->udi_serial));
 	if (dev->serial != NULL)
-		strlcpy(di->udi_serial, dev->serial,
-		    sizeof(di->udi_serial));
+		strlcpy(di->udi_serial, dev->serial, sizeof(di->udi_serial));
 }
 
 int