linux-kernel - Re: [PATCH v1] usb: gadget: add WebUSB support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 1 Jan 2023 11:21:33 -0500
From:   Alan Stern <stern@...land.harvard.edu>
To:     Jó Ágila Bitsch <jgilab@...il.com>
Cc:     Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org
Subject: Re: [PATCH v1] usb: gadget: add WebUSB support

On Sat, Dec 31, 2022 at 09:26:44PM +0100, Jó Ágila Bitsch wrote:
> On Sat, Dec 31, 2022 at 8:47 PM Alan Stern <stern@...land.harvard.edu> wrote:
> >
> > On Fri, Dec 30, 2022 at 09:11:43PM +0100, Jó Ágila Bitsch wrote:
> > >
> > > During device enumeration, a host recognizes that the announced
> > > USB version is at least 2.01, which means, that there are BOS
> >
> > Where is this 2.01 value specified?  I don't remember seeing it in the
> > usual USBIF documents.
> 
> This is actually from the backport of BOS descriptors to USB 2
> 
> Citing Randy Aull from
> https://community.osr.com/discussion/comment/249742/#Comment_249742
> >  There is no specification called USB 2.1, however there is such a thing as a USB 2.1 device.
> > The USB 2.0 ECN for LPM introduced a new descriptor request to the enumeration process
> > for USB 2 devices (the BOS descriptor set). The problem is that software can't request a new
> > descriptor type to old devices that don't understand it without introducing compatibility issues
> > with some devices (more than you would probably expect). So, software needed a way to
> > identify whether a device could support the host requesting a BOS descriptor set. The solution
> > in this ECN was to require devices supporting the ECN to set their bcdUSB to 0x0201 (2.01).
> >
> > Now, when we created the USB 3.0 spec, we again wanted to leverage the BOS descriptor, not
> > only because we wanted to mandate USB 2 LPM in 3.0 devices when operating at high-speed,
> > but also so the device can indicate to a host that it can operate at SuperSpeed (to support
> > everyone's favorite "your device can perform faster" popup). Knowing that when operating at
> > high-speed these devices needed to report the BOS descriptor set, we knew that it couldn't
> > set the bcdUSB to 0x0200. We mistakenly wrote it down as 0x0210 instead of 0x0201 in the
> > 3.0 spec (perhaps we just said "two point one" which might have been ambiguous) when we
> > were trying to just be consistent with the requirement from the LPM ECN. So, rather than
> > changing it back to 0x0201 in the USB 3.0 spec, we just ran with it.
> >
> > So, there are USB 2.0 devices, USB 2.01 devices and USB 2.1 devices, even though the latest
> > revision of the USB 2 spec is USB 2.0. I have recommended that the USB-IF actually create a
> > USB 2.1 specification that captures all of the various errata, ECNs, etc from over the years, but
> > it hasn't happened yet.

Interesting history.  And now that you point it out, I do see at the end 
of section 3 of the USB 2.0 Link Power Management ECN:

	Devices that support the BOS descriptor must have a bcdUSB value 
	of 0201H or larger.

> Btw, configfs already includes these version codes to support Link
> Power Management (LPM) and
> the associated BOS descriptor, so I didn't add that part, I only added
> webusb as a condition as to
> when to send BOS descriptors.

Right.

> > > @@ -713,14 +714,16 @@ static int bos_desc(struct usb_composite_dev *cdev)
> > >        * A SuperSpeed device shall include the USB2.0 extension descriptor
> > >        * and shall support LPM when operating in USB2.0 HS mode.
> > >        */
> > > -     usb_ext = cdev->req->buf + le16_to_cpu(bos->wTotalLength);
> > > -     bos->bNumDeviceCaps++;
> > > -     le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
> > > -     usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
> > > -     usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> > > -     usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
> > > -     usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT |
> > > -                                         USB_BESL_SUPPORT | besl);
> > > +     if (cdev->gadget->lpm_capable) {
> >
> > This change doesn't seem to be related to the purpose of this patch.
> 
> Actually, it is.
> 
> Previously, BOS descriptors would only ever be sent if the device was
> lpm_capable.
> For this reason, this descriptor was previously sent unconditionally.
> 
> With my patch in place, it could be that a device is not lpm_capable, but still
> wants to send BOS descriptors to announce its WebUSB capability,
> therefore I added
> this condition.

Okay.  It would be good to explain this in the patch description.

Alan Stern