There doesn't seem to be much interest in this problem, so I'll just use
this change locally to solve my issue.
Though isn't it a tiny buffer overflow if a malicious or broken USB
device sends a bLength of 255 and actually follows through with it?
usb_string_descriptor_t is only 254 bytes.
On Sun, Aug 21, 2022 at 12:35:00AM -0400, bug wrote:
> I got a StarTech SV431USBDDM KVM switch, and it specifically mangles the
> string descriptor responses for my keyboard (which otherwise works just
> fine) when probed about the vendor string after plugging in or resetting
> the device.
>
> After a ton of messing around, I also found out that this problem just
> isn't happening at all if OpenBSD only sends a single string descriptor
> request with the maximum possible length, rather than two as it
> currently does.
>
> According to the USB 2.0 and 3.2 specs, devices that receive a larger
> wLength than the descriptor's actual length /should/ just send the full
> descriptor in a short packet. Of course, the current behavior isn't
> wrong either, if the wLength is shorter, then only the initial bytes or
> length (depending on the spec) /should/ be sent.
>
> (So unless there's some deeper bug in OpenBSD, my KVM switch is totally
> dropping the ball either way. I'll probably post about it on Misc later,
> unless details are desired here...)
>
> But I'm curious if there's a reason for sending two separate packets
> when one should be fine. Do certain USB devices expect it that way?
>
> I suppose it seems Windows does it this way too (the only reason the
> switch works there is because they neglect to check the vendor string),
> so the answer to that could very well be a hard yes...
>
> This is my first post, so sorry in advance if I did anything dumb. Diff
> provided for context.
>
>
> Index: usb_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
> retrieving revision 1.158
> diff -u -p -r1.158 usb_subr.c
> --- usb_subr.c 16 Feb 2022 06:23:42 -0000 1.158
> +++ usb_subr.c 21 Aug 2022 03:33:10 -0000
> @@ -125,20 +125,17 @@ usbd_get_string_desc(struct usbd_device
> req.bRequest = UR_GET_DESCRIPTOR;
> USETW2(req.wValue, UDESC_STRING, sindex);
> USETW(req.wIndex, langid);
> - USETW(req.wLength, 2); /* size and descriptor type first */
> + USETW(req.wLength, sizeof(*sdesc)); /* size and descriptor type
> first */
> +
> err = usbd_do_request_flags(dev, &req, sdesc, USBD_SHORT_XFER_OK,
> &actlen, USBD_DEFAULT_TIMEOUT);
> +
> if (err)
> return (err);
>
> if (actlen < 2)
> return (USBD_SHORT_XFER);
>
> - USETW(req.wLength, sdesc->bLength); /* the whole string */
> - err = usbd_do_request_flags(dev, &req, sdesc, USBD_SHORT_XFER_OK,
> - &actlen, USBD_DEFAULT_TIMEOUT);
> - if (err)
> - return (err);
>
> if (actlen != sdesc->bLength) {
> DPRINTFN(-1, ("%s: expected %d, got %d\n", __func__,
>
