| 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
| ||
|
Message-ID: <CAB0kiBK-G3fyaX6TmS008nDyQ+x1zCZpwe_RT-sNdh78gQeOAQ@mail.gmail.com>
Date: Wed, 31 Jul 2024 20:09:59 -0400
From: Chris Wulff <crwulff@...il.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Chris Wulff <Chris.Wulff@...mp.com>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, Christian Brauner <brauner@...nel.org>,
Paul Cercueil <paul@...pouillou.net>, Jan Kara <jack@...e.cz>, Jeff Layton <jlayton@...nel.org>,
Dmitry Antipov <dmantipov@...dex.ru>, David Sands <david.sands@...mp.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>
Subject: Re: [PATCH v3] usb: gadget: f_fs: add capability for dfu run-time descriptor
On Wed, Jul 31, 2024 at 4:28 AM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> On Wed, Apr 24, 2024 at 10:14:58PM +0000, Chris Wulff wrote:
> > From: David Sands <david.sands@...mp.com>
> >
> > Add the ability for FunctionFS driver to be able to create DFU Run-Time
> > descriptors.
>
> Don't you need some userspace documentation for this as well?
Yes, I will add some.
>
> > --- a/include/uapi/linux/usb/ch9.h
> > +++ b/include/uapi/linux/usb/ch9.h
> > @@ -254,6 +254,9 @@ struct usb_ctrlrequest {
> > #define USB_DT_DEVICE_CAPABILITY 0x10
> > #define USB_DT_WIRELESS_ENDPOINT_COMP 0x11
> > #define USB_DT_WIRE_ADAPTER 0x21
> > +/* From USB Device Firmware Upgrade Specification, Revision 1.1 */
> > +#define USB_DT_DFU_FUNCTIONAL 0x21
>
> So USB_DT_WIRE_ADAPTER and USB_DT_DFU_FUNCTIONAL are the same? That
> seems wrong.
>
> > +/* these are from the Wireless USB spec */
>
> What spec? What "these"?
I inserted the DFU constant in numerical order, splitting the original
section, so I
duplicated the comment. (Partly to make it obvious that there were two with the
same number.) It looks like possibly wireless usb is a dead spec.
You can find wireless usb v1.0 on usb.org in the wayback machine, but it looks
like most of the references have been purged from the current site, and have
been gone for a few years. There was apparently a v1.1 too, but I never found
a copy of that anywhere.
https://web.archive.org/web/20090325042850/http://www.usb.org/developers/wusb/docs/
A few references remain but say the specification is no longer available. Eg.
https://www.usb.org/bos-descriptor-types
The original section of code that I inserted the new constant into was:
/* these are from the Wireless USB spec */
#define USB_DT_SECURITY 0x0c
#define USB_DT_KEY 0x0d
#define USB_DT_ENCRYPTION_TYPE 0x0e
#define USB_DT_BOS 0x0f
#define USB_DT_DEVICE_CAPABILITY 0x10
#define USB_DT_WIRELESS_ENDPOINT_COMP 0x11
#define USB_DT_WIRE_ADAPTER 0x21
#define USB_DT_RPIPE 0x22
#define USB_DT_CS_RADIO_CONTROL 0x23
/* From the T10 UAS specification */
>
> > #define USB_DT_RPIPE 0x22
> > #define USB_DT_CS_RADIO_CONTROL 0x23
> > /* From the T10 UAS specification */
> > @@ -263,6 +266,7 @@ struct usb_ctrlrequest {
> > /* From the USB 3.1 spec */
> > #define USB_DT_SSP_ISOC_ENDPOINT_COMP 0x31
> >
> > +
> > /* Conventional codes for class-specific descriptors. The convention is
> > * defined in the USB "Common Class" Spec (3.11). Individual class specs
> > * are authoritative for their usage, not the "common class" writeup.
>
> Unneeded change?
Yeah, I'll clean that up.
>
> > @@ -329,9 +333,10 @@ struct usb_device_descriptor {
> > #define USB_CLASS_USB_TYPE_C_BRIDGE 0x12
> > #define USB_CLASS_MISC 0xef
> > #define USB_CLASS_APP_SPEC 0xfe
> > -#define USB_CLASS_VENDOR_SPEC 0xff
> > +#define USB_SUBCLASS_DFU 0x01
> >
> > -#define USB_SUBCLASS_VENDOR_SPEC 0xff
> > +#define USB_CLASS_VENDOR_SPEC 0xff
> > +#define USB_SUBCLASS_VENDOR_SPEC 0xff
>
> Why reorder these?
The subclasses are specific to the class they are under.
USB_SUBCLASS_DFU is part of USB_CLASS_APP_SPEC
USB_SUBCLASS_VENDOR_SPEC is part of USB_CLASS_VENDOR_SPEC
>
> >
> > /*-------------------------------------------------------------------------*/
> >
> > diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
> > index 9f88de9c3d66..6d2061500184 100644
> > --- a/include/uapi/linux/usb/functionfs.h
> > +++ b/include/uapi/linux/usb/functionfs.h
> > @@ -37,6 +37,31 @@ struct usb_endpoint_descriptor_no_audio {
> > __u8 bInterval;
> > } __attribute__((packed));
> >
> > +/**
> > + * struct usb_dfu_functional_descriptor - DFU Functional descriptor
> > + * @bLength: Size of the descriptor (bytes)
> > + * @bDescriptorType: USB_DT_DFU_FUNCTIONAL
> > + * @bmAttributes: DFU attributes
> > + * @wDetachTimeOut: Maximum time to wait after DFU_DETACH (ms, le16)
> > + * @wTransferSize: Maximum number of bytes per control-write (le16)
> > + * @bcdDFUVersion: DFU Spec version (BCD, le16)
> > + */
> > +struct usb_dfu_functional_descriptor {
> > + __u8 bLength;
> > + __u8 bDescriptorType;
> > + __u8 bmAttributes;
> > + __le16 wDetachTimeOut;
> > + __le16 wTransferSize;
> > + __le16 bcdDFUVersion;
> > +} __attribute__ ((packed));
> > +
> > +/* from DFU functional descriptor bmAttributes */
> > +#define DFU_FUNC_ATT_WILL_DETACH (1 << 3)
> > +#define DFU_FUNC_ATT_MANIFEST_TOLERANT (1 << 2)
> > +#define DFU_FUNC_ATT_CAN_UPLOAD (1 << 1)
> > +#define DFU_FUNC_ATT_CAN_DOWNLOAD (1 << 0)
>
> Please use proper BIT macros here to make this more obvious. And in
> sorted order?
Ok. I'll fix this up
>
> thanks,
>
> greg k-h
>
Powered by blists - more mailing lists