usb: Export BOS descriptor to sysfs
PATCH 1/1 usb: Export BOS descriptor to sysfs
Motivation
----------
The kernel already retrieves and caches the binary device object store (BOS) descriptor from USB devices it enumerates. Export this descriptor to userspace via sysfs, so users do not need to open the USB device with the correct permissions and requesting the descriptor themselves.
A BOS descriptor contains a set of device capability descriptors. One that is of interest to users is the platform descriptor. This contains a 128-bit UUID and arbitrary data. The descriptor allows parties outside of USB-IF to add additional metadata about a device in a standards-compliant manner.
Notable examples include the WebUSB and Microsoft OS 2.0 descriptors. Of course, there could be more. By exporting the entire BOS descriptor we can handle these and all future device capabilities. In addition, tools like udev can match rules on device capabilities in the BOS without requiring additional I/O with the USB device.
Implementation
--------------
Add bos_descriptor file to sysfs. This is a binary file and it works the same way as the existing descriptors file. The file exists even if a device does not have a BOS descriptor (the file will be empty in this case). This allows users to detect if the kernel supports reading the BOS via sysfs and fall back to direct USB I/O if needed.
--- Documentation/ABI/testing/sysfs-bus-usb | 9 +++++++ drivers/usb/core/sysfs.c | 35 ++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-)
Documentation/ABI/testing/sysfs-bus-usb +9 -0
@@ -293,3 +293,12 @@ Description: USB 3.2 adds Dual-lane support, 2 rx and 2 tx -lanes over Type-C. Inter-Chip SSIC devices support asymmetric lanes up to 4 lanes per direction. Devices before USB 3.2 are single lane (tx_lanes = 1) + +What: /sys/bus/usb/devices/.../bos_descriptor +Date: March 2024 +Contact: Elbert Mai <[email protected]> +Description: + Binary file containing the cached binary device object store (BOS) + descriptor of the device. This file is empty if the BOS descriptor + is not present. The kernel will not request a BOS descriptor from + the device if its bcdUSB value is less than 0x0201.
drivers/usb/core/sysfs.c +34 -1
@@ -901,7 +901,7 @@ read_descriptors(struct file *filp, struct kobject *kobj, srclen =sizeof (struct usb_device_descriptor ); }else { src = udev->rawdescriptors[cfgno]; - srclen =__le16_to_cpu (udev->config[cfgno].desc. + srclen =le16_to_cpu (udev->config[cfgno].desc. wTotalLength); }if (off < srclen) {
@@ -923,6 +923,34 @@ static struct bin_attribute dev_bin_attr_descriptors = { .size = 18 + 65535, /* dev descr + max-size raw descriptor */ }; +static ssize_t +read_bos_descriptor (struct file *filp,struct kobject *kobj, +struct bin_attribute *attr, +char *buf,loff_t off,size_t count) +{ +struct device *dev =kobj_to_dev (kobj); +struct usb_device *udev =to_usb_device (dev); +struct usb_host_bos *bos = udev->bos; +struct usb_bos_descriptor *desc; +size_t desclen, n =0 ; + +if (bos) { + desc = bos->desc; + desclen =le16_to_cpu (desc->wTotalLength); +if (off < desclen) { + n =min (count, desclen - (size_t ) off); +memcpy (buf, (void *) desc + off, n); + } + } +return n; +} + +static struct bin_attribute dev_bin_attr_bos_descriptor = { + .attr = {.name ="bos_descriptor" , .mode =0444 }, + .read = read_bos_descriptor, + .size =65535 ,/* max-size BOS descriptor */ +}; +/* * Show & store the current value of authorized_default */
@@ -1042,6 +1070,10 @@ int usb_create_sysfs_dev_files(struct usb_device *udev)if (retval) goto error; + retval =device_create_bin_file (dev, &dev_bin_attr_bos_descriptor); +if (retval) + goto error; + retval =add_persist_attributes (dev);if (retval) goto error;
@@ -1071,6 +1103,7 @@ void usb_remove_sysfs_dev_files(struct usb_device *udev)remove_power_attributes (dev);remove_persist_attributes (dev); +device_remove_bin_file (dev, &dev_bin_attr_bos_descriptor);device_remove_bin_file (dev, &dev_bin_attr_descriptors); }
Signature · 1 line
2.34.1
Greg KH
maintainerQuoted 13 lines · Elbert Mai
Motivation
----------The kernel already retrieves and caches the binary device object store (BOS) descriptor from USB devices it enumerates. Export this descriptor to userspace via sysfs, so users do not need to open the USB device with the correct permissions and requesting the descriptor themselves.
A BOS descriptor contains a set of device capability descriptors. One that is of interest to users is the platform descriptor. This contains a 128-bit UUID and arbitrary data. The descriptor allows parties outside of USB-IF to add additional metadata about a device in a standards-compliant manner.
Quoted 5 lines
Notable examples include the WebUSB and Microsoft OS 2.0 descriptors. Of course, there could be more. By exporting the entire BOS descriptor we can handle these and all future device capabilities. In addition, tools like udev can match rules on device capabilities in the BOS without requiring additional I/O with the USB device.
But this requires that userspace parse the raw descriptor, right? Why not also export the descriptor information in simpler form through different sysfs files as well so that scripts can read this information easily?
That's not to say we don't also want this binary file, just maybe add more?
Quoted 8 lines
Implementation
--------------Add
bos_descriptorfile to sysfs. This is a binary file and it works the same way as the existing descriptors file. The file exists even if a device does not have a BOS descriptor (the file will be empty in this case). This allows users to detect if the kernel supports reading the BOS via sysfs and fall back to direct USB I/O if needed.
This is nice, but can you only create the file if it actually is present? Use the is_visible callback to determine this, having an "empty" binary sysfs file isn't that nice.
Other tiny review comments below:
Quoted 28 lines
Signed-off-by Elbert Mai <[email protected]>--- Documentation/ABI/testing/sysfs-bus-usb | 9 +++++++ drivers/usb/core/sysfs.c | 35 ++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-)Documentation/ABI/testing/sysfs-bus-usb +9
-0@@ -293,3 +293,12 @@ Description: USB 3.2 adds Dual-lane support, 2 rx and 2 tx -lanes over Type-C. Inter-Chip SSIC devices support asymmetric lanes up to 4 lanes per direction. Devices before USB 3.2 are single lane (tx_lanes = 1) + +What: /sys/bus/usb/devices/.../bos_descriptor +Date: March 2024 +Contact: Elbert Mai <[email protected]> +Description: + Binary file containing the cached binary device object store (BOS) + descriptor of the device. This file is empty if the BOS descriptor + is not present. The kernel will not request a BOS descriptor from + the device if its bcdUSB value is less than 0x0201.drivers/usb/core/sysfs.c +0
-0
drivers/usb/core/sysfs.c read_descriptors(struct file *filp, struct kobject *kobj, · line 904
Quoted 5 lines
drivers/usb/core/sysfs.c +1
-1@@ -901,7 +901,7 @@ read_descriptors(struct file *filp, struct kobject *kobj, srclen =sizeof (struct usb_device_descriptor ); }else { src = udev->rawdescriptors[cfgno]; - srclen = __le16_to_cpu(udev->config[cfgno].desc. + srclen = le16_to_cpu(udev->config[cfgno].desc.
Why is this line changed?
Quoted 17 lines
drivers/usb/core/sysfs.c +10
-0wTotalLength); }if (off < srclen) {@@ -923,6 +923,34 @@ static struct bin_attribute dev_bin_attr_descriptors = { .size = 18 + 65535, /* dev descr + max-size raw descriptor */ }; +static ssize_t +read_bos_descriptor (struct file *filp,struct kobject *kobj, +struct bin_attribute *attr, +char *buf,loff_t off,size_t count) +{ +struct device *dev =kobj_to_dev (kobj); +struct usb_device *udev =to_usb_device (dev); +struct usb_host_bos *bos = udev->bos; +struct usb_bos_descriptor *desc; +size_t desclen, n =0 ;
drivers/usb/core/sysfs.c static struct bin_attribute dev_bin_attr_descriptors = { · line 945
Quoted 10 lines
drivers/usb/core/sysfs.c +10
-0+ +if (bos) { + desc = bos->desc; + desclen =le16_to_cpu (desc->wTotalLength); +if (off < desclen) { + n =min (count, desclen - (size_t ) off); +memcpy (buf, (void *) desc + off, n); + } + } +return n;
If bos is not present an error should return, not 0, right?
drivers/usb/core/sysfs.c static struct bin_attribute dev_bin_attr_descriptors = { · line 950
Quoted 5 lines
drivers/usb/core/sysfs.c +5
-0+} + +static struct bin_attribute dev_bin_attr_bos_descriptor = { + .attr = {.name ="bos_descriptor" , .mode =0444 }, + .read = read_bos_descriptor,
Isn't there a mscro for all of this?
drivers/usb/core/sysfs.c static struct bin_attribute dev_bin_attr_descriptors = { · line 951
Quoted 1 line
drivers/usb/core/sysfs.c +1
-0+ .size = 65535, /* max-size BOS descriptor */
Do we know the size of the descriptor at runtime? Ideally we would set it to be that, not an unknown size that way userspace knows what needs to be read. But that's not really a big deal, if it's not possible, as the other USB binary descriptor does much the same.
drivers/usb/core/sysfs.c static struct bin_attribute dev_bin_attr_descriptors = { · line 952
Quoted 1 line
drivers/usb/core/sysfs.c +1
-0+};
This should go into an attribute group with the other descriptor, making the rest of the patch not needed and simpler.
drivers/usb/core/sysfs.c int usb_create_sysfs_dev_files(struct usb_device *udev) · line 1076
Quoted 12 lines
drivers/usb/core/sysfs.c +5
-0+/* * Show & store the current value of authorized_default */ @@ -1042,6 +1070,10 @@ int usb_create_sysfs_dev_files(struct usb_device *udev)if (retval) goto error; + retval =device_create_bin_file (dev, &dev_bin_attr_bos_descriptor); +if (retval) + goto error; +
long-term I'd like to get rid of the individual "add and remove" sysfs files in this function, and just use the default groups instead so the driver core handles it. It's not an issue here for this change, but if you can find a way to work on that as part of a patch series here, I think that would be really good (if possible, not a requirement.)
thanks!
greg k-h
Quoted 20 lines · Greg KH
Quoted 13 lines · Elbert Mai
Motivation
----------The kernel already retrieves and caches the binary device object store (BOS) descriptor from USB devices it enumerates. Export this descriptor to userspace via sysfs, so users do not need to open the USB device with the correct permissions and requesting the descriptor themselves.
A BOS descriptor contains a set of device capability descriptors. One that is of interest to users is the platform descriptor. This contains a 128-bit UUID and arbitrary data. The descriptor allows parties outside of USB-IF to add additional metadata about a device in a standards-compliant manner.
Quoted 5 lines
Notable examples include the WebUSB and Microsoft OS 2.0 descriptors. Of course, there could be more. By exporting the entire BOS descriptor we can handle these and all future device capabilities. In addition, tools like udev can match rules on device capabilities in the BOS without requiring additional I/O with the USB device.
Quoted 7 lines
But this requires that userspace parse the raw descriptor, right? Why not also export the descriptor information in simpler form through different sysfs files as well so that scripts can read this information easily?
That's not to say we don't also want this binary file, just maybe add more?
I've thought about this for a while, but IMHO it doesn't seem possible to present the BOS in a simpler form like how it's done for the device and configuration descriptors. The BOS was explicitly designed to be flexible and extensible, and it's hard to make a "rich" sysfs interface around that.
We could have a platform_uuids file that lists all platform UUIDs in the BOS. These could be presented in RFC 4122 string format, each separated by some delimiter. This would make it possible to write a udev rule that matches a particular UUID via a glob pattern. This seems like a rather inelegant way of handling this, IMHO.
With regards to parsing raw descriptors in userspace, this already happens at least with udev. About a dozen files in /usr/lib/udev/rules.d invoke a built-in program called "usb_id", which opens the descriptors binary file (among other files in sysfs) and parses it for interface information.
I think users and scripts interested in reading the BOS are best served with userspace tools that can parse the BOS and present it in a convenient textual format of their choosing. Having bos_descriptor in sysfs makes this possible without needing elevated permissions or USB I/O. But I could be mistaken in this regard.
Quoted 9 lines
Quoted 8 lines
Implementation
--------------Add
bos_descriptorfile to sysfs. This is a binary file and it works the same way as the existing descriptors file. The file exists even if a device does not have a BOS descriptor (the file will be empty in this case). This allows users to detect if the kernel supports reading the BOS via sysfs and fall back to direct USB I/O if needed.
Quoted 3 lines
This is nice, but can you only create the file if it actually is present? Use the
is_visiblecallback to determine this, having an "empty" binary sysfs file isn't that nice.
Of course, I can make that happen.
Quoted 31 lines
Other tiny review comments below:
Quoted 29 lines
Signed-off-by Elbert Mai <[email protected]>--- Documentation/ABI/testing/sysfs-bus-usb | 9 +++++++ drivers/usb/core/sysfs.c | 35 ++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-)Documentation/ABI/testing/sysfs-bus-usb +9
-0@@ -293,3 +293,12 @@ Description: USB 3.2 adds Dual-lane support, 2 rx and 2 tx -lanes over Type-C. Inter-Chip SSIC devices support asymmetric lanes up to 4 lanes per direction. Devices before USB 3.2 are single lane (tx_lanes = 1) + +What: /sys/bus/usb/devices/.../bos_descriptor +Date: March 2024 +Contact: Elbert Mai <[email protected]> +Description: + Binary file containing the cached binary device object store (BOS) + descriptor of the device. This file is empty if the BOS descriptor + is not present. The kernel will not request a BOS descriptor from + the device if its bcdUSB value is less than 0x0201.drivers/usb/core/sysfs.c +0
-0@@ -901,7 +901,7 @@ read_descriptors(struct file *filp, struct kobject
Quoted 8 lines
drivers/usb/core/sysfs.c read_descriptors(struct file *filp, struct kobject *kobj, · line 904
Quoted 6 lines
drivers/usb/core/sysfs.c +1
-1*kobj, srclen =sizeof (struct usb_device_descriptor ); }else { src = udev->rawdescriptors[cfgno]; - srclen = __le16_to_cpu(udev->config[cfgno].desc. + srclen = le16_to_cpu(udev->config[cfgno].desc.Why is this line changed?
Consistency with the rest of the file. This was the only instance of __le16_to_cpu, other lines used le16_to_cpu (no underscores).
Quoted 22 lines
Quoted 22 lines
drivers/usb/core/sysfs.c +0
-0wTotalLength); }drivers/usb/core/sysfs.c +14
-0if (off < srclen) {@@ -923,6 +923,34 @@ static struct bin_attribute dev_bin_attr_descriptors = { .size = 18 + 65535, /* dev descr + max-size raw descriptor */ }; +static ssize_t +read_bos_descriptor (struct file *filp,struct kobject *kobj, +struct bin_attribute *attr, +char *buf,loff_t off,size_t count) +{ +struct device *dev =kobj_to_dev (kobj); +struct usb_device *udev =to_usb_device (dev); +struct usb_host_bos *bos = udev->bos; +struct usb_bos_descriptor *desc; +size_t desclen, n =0 ; + +if (bos) { + desc = bos->desc; + desclen =le16_to_cpu (desc->wTotalLength);
Quoted 8 lines
drivers/usb/core/sysfs.c static struct bin_attribute dev_bin_attr_descriptors = { · line 945
Quoted 6 lines
drivers/usb/core/sysfs.c +6
-0+if (off < desclen) { + n =min (count, desclen - (size_t ) off); +memcpy (buf, (void *) desc + off, n); + } + } +return n;If bos is not present an error should return, not 0, right?
The interface_show function in sysfs.c returns 0 if the string describing the interface is not present, so I did it the same way. Should I instead return -ENOENT or some other specific error code?
Quoted 7 lines
drivers/usb/core/sysfs.c static struct bin_attribute dev_bin_attr_descriptors = { · line 950
Quoted 5 lines
drivers/usb/core/sysfs.c +5
-0+} + +static struct bin_attribute dev_bin_attr_bos_descriptor = { + .attr = {.name ="bos_descriptor" , .mode =0444 }, + .read = read_bos_descriptor,Isn't there a mscro for all of this?
It turns out there is, called BIN_ATTR_RO in in linux/sysfs.h. I'm not sure why the original code didn't use it for read_descriptors. I can change both instances to shorten the code.
Quoted 6 lines
drivers/usb/core/sysfs.c static struct bin_attribute dev_bin_attr_descriptors = { · line 951
Quoted 1 line
drivers/usb/core/sysfs.c +1
-0+ .size = 65535, /* max-size BOS descriptor */Do we know the size of the descriptor at runtime? Ideally we would set it to be that, not an unknown size that way userspace knows what needs to be read. But that's not really a big deal, if it's not possible, as the other USB binary descriptor does much the same.
We do know the size via wTotalLength, but that varies between USB devices and dev_bin_attr_bos_descriptor is a static variable. So not really possible unless we dynamically allocate all the attribute structs.
Quoted 4 lines
drivers/usb/core/sysfs.c static struct bin_attribute dev_bin_attr_descriptors = { · line 952
Quoted 1 line
drivers/usb/core/sysfs.c +1
-0+};This should go into an attribute group with the other descriptor, making the rest of the patch not needed and simpler.
Will do. I'm in favor of simpler patches.
Quoted 9 lines
Quoted 9 lines
drivers/usb/core/sysfs.c +1
-0+/* * Show & store the current value of authorized_default */
drivers/usb/core/sysfs.c +0
-0@@ -1042,6 +1070,10 @@ int usb_create_sysfs_dev_files(struct usb_device *udev)if (retval) goto error;
Quoted 10 lines
drivers/usb/core/sysfs.c int usb_create_sysfs_dev_files(struct usb_device *udev) · line 1076
Quoted 4 lines
drivers/usb/core/sysfs.c +4
-0+ retval =device_create_bin_file (dev, &dev_bin_attr_bos_descriptor); +if (retval) + goto error; +long-term I'd like to get rid of the individual "add and remove" sysfs files in this function, and just use the default groups instead so the driver core handles it. It's not an issue here for this change, but if you can find a way to work on that as part of a patch series here, I think that would be really good (if possible, not a requirement.)
I'll see what I can do. I'll go for at least the low-hanging fruit.
Quoted 3 lines
thanks!
greg k-h
And thank you for taking the time to review this patch!
-Elbert