From: Mark Kettenis Subject: Re: To: "Theo de Raadt" Cc: tech@openbsd.org, djm@openbsd.org Date: Mon, 07 Sep 2026 20:58:19 +0200 > From: "Theo de Raadt" > Date: Mon, 07 Sep 2026 12:50:45 -0600 > > Theo de Raadt wrote: > > > Kirill A. Korinsky 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: That's much better. > 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 > >