diff mbox series

[v2] usb: Export BOS descriptor to sysfs

Message ID 20240305002301.95323-1-code@elbertmai.com (mailing list archive)
State Accepted
Commit 12fc84e8c4288cc8ed5f14a35e077130c2cfece2
Headers show
Series [v2] usb: Export BOS descriptor to sysfs | expand

Commit Message

Elbert Mai March 5, 2024, 12:23 a.m. UTC
Motivation
----------

The binary device object store (BOS) of a USB device consists of the BOS
descriptor followed by 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, and it allows parties outside of USB-IF to add
additional metadata about a USB device in a standards-compliant manner.
Notable examples include the WebUSB and Microsoft OS 2.0 descriptors.

The kernel already retrieves and caches the BOS from USB devices if its
bcdUSB is >= 0x0201. Because the BOS is flexible and extensible, we export
the entire BOS to sysfs so users can retrieve whatever device capabilities
they desire, without requiring USB I/O or elevated permissions.

Implementation
--------------

Add bos_descriptors attribute to sysfs. This is a binary file and it works
the same way as the existing descriptors attribute. The file exists only if
the BOS is present in the USB device.

Also create a binary attribute group, so the driver core can handle the
creation of both the descriptors and bos_descriptors attributes in sysfs.

Signed-off-by: Elbert Mai <code@elbertmai.com>
---
Changes in v2:
 - Rename to bos_descriptors (plural) since the attribute contains the
   entire BOS, not just the first descriptor in it.
 - Use binary attribute groups to let driver core handle attribute
   creation for both descriptors and bos_descriptors.
 - The attribute is visible in sysfs only if the BOS is present in the
   USB device.

 Documentation/ABI/testing/sysfs-bus-usb | 10 ++++
 drivers/usb/core/sysfs.c                | 78 +++++++++++++++++++------
 2 files changed, 71 insertions(+), 17 deletions(-)

Comments

Greg KH March 5, 2024, 7:52 a.m. UTC | #1
On Mon, Mar 04, 2024 at 04:23:01PM -0800, Elbert Mai wrote:
> Motivation
> ----------
> 
> The binary device object store (BOS) of a USB device consists of the BOS
> descriptor followed by 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, and it allows parties outside of USB-IF to add
> additional metadata about a USB device in a standards-compliant manner.
> Notable examples include the WebUSB and Microsoft OS 2.0 descriptors.
> 
> The kernel already retrieves and caches the BOS from USB devices if its
> bcdUSB is >= 0x0201. Because the BOS is flexible and extensible, we export
> the entire BOS to sysfs so users can retrieve whatever device capabilities
> they desire, without requiring USB I/O or elevated permissions.
> 
> Implementation
> --------------
> 
> Add bos_descriptors attribute to sysfs. This is a binary file and it works
> the same way as the existing descriptors attribute. The file exists only if
> the BOS is present in the USB device.
> 
> Also create a binary attribute group, so the driver core can handle the
> creation of both the descriptors and bos_descriptors attributes in sysfs.
> 
> Signed-off-by: Elbert Mai <code@elbertmai.com>
> ---
> Changes in v2:
>  - Rename to bos_descriptors (plural) since the attribute contains the
>    entire BOS, not just the first descriptor in it.
>  - Use binary attribute groups to let driver core handle attribute
>    creation for both descriptors and bos_descriptors.
>  - The attribute is visible in sysfs only if the BOS is present in the
>    USB device.

Very nice, thanks for this!

One very minor comment, you can send a follow-on patch for this if you
like:

> +static umode_t dev_bin_attrs_are_visible(struct kobject *kobj,
> +		struct bin_attribute *a, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct usb_device *udev = to_usb_device(dev);
> +
> +	/* All USB devices have a device descriptor, so the descriptors
> +	 * attribute always exists. No need to check for its visibility.
> +	 */

This comment is in the "wrong" style, I think checkpatch will complain
about that, right?

But it's a bit confusing, you say "no need to check", and then you:

> +	if (a == &bin_attr_bos_descriptors) {
> +		if (udev->bos == NULL)
> +			return 0;
> +	}

check something :)

How about this as a comment instead:
	/*
	 * If this is the BOS descriptor, check to verify if the device
	 * has that descriptor at all or not.
	 */

That's all you need here, right?

Anyway, again, very nice, I'll go queue this up and run it through the
0-day tests.

thanks!

greg k -h
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb
index 614d216dff1d..102ee4215e48 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -293,3 +293,13 @@  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_descriptors
+Date:		March 2024
+Contact:	Elbert Mai <code@elbertmai.com>
+Description:
+		Binary file containing the cached binary device object store (BOS)
+		of the device. This consists of the BOS descriptor followed by the
+		set of device capability descriptors. All descriptors read from
+		this file are in bus-endian format. Note that the kernel will not
+		request the BOS from a device if its bcdUSB is less than 0x0201.
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index a2ca38e25e0c..a50b6f496eb6 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -870,16 +870,10 @@  static struct attribute_group dev_string_attr_grp = {
 	.is_visible =	dev_string_attrs_are_visible,
 };
 
-const struct attribute_group *usb_device_groups[] = {
-	&dev_attr_grp,
-	&dev_string_attr_grp,
-	NULL
-};
-
 /* Binary descriptors */
 
 static ssize_t
-read_descriptors(struct file *filp, struct kobject *kobj,
+descriptors_read(struct file *filp, struct kobject *kobj,
 		struct bin_attribute *attr,
 		char *buf, loff_t off, size_t count)
 {
@@ -901,7 +895,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) {
@@ -916,11 +910,66 @@  read_descriptors(struct file *filp, struct kobject *kobj,
 	}
 	return count - nleft;
 }
+static BIN_ATTR_RO(descriptors, 18 + 65535); /* dev descr + max-size raw descriptor */
+
+static ssize_t
+bos_descriptors_read(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 BIN_ATTR_RO(bos_descriptors, 65535); /* max-size BOS */
 
-static struct bin_attribute dev_bin_attr_descriptors = {
-	.attr = {.name = "descriptors", .mode = 0444},
-	.read = read_descriptors,
-	.size = 18 + 65535,	/* dev descr + max-size raw descriptor */
+/* When modifying this list, be sure to modify dev_bin_attrs_are_visible()
+ * accordingly.
+ */
+static struct bin_attribute *dev_bin_attrs[] = {
+	&bin_attr_descriptors,
+	&bin_attr_bos_descriptors,
+	NULL
+};
+
+static umode_t dev_bin_attrs_are_visible(struct kobject *kobj,
+		struct bin_attribute *a, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct usb_device *udev = to_usb_device(dev);
+
+	/* All USB devices have a device descriptor, so the descriptors
+	 * attribute always exists. No need to check for its visibility.
+	 */
+	if (a == &bin_attr_bos_descriptors) {
+		if (udev->bos == NULL)
+			return 0;
+	}
+	return a->attr.mode;
+}
+
+static const struct attribute_group dev_bin_attr_grp = {
+	.bin_attrs =		dev_bin_attrs,
+	.is_bin_visible =	dev_bin_attrs_are_visible,
+};
+
+const struct attribute_group *usb_device_groups[] = {
+	&dev_attr_grp,
+	&dev_string_attr_grp,
+	&dev_bin_attr_grp,
+	NULL
 };
 
 /*
@@ -1038,10 +1087,6 @@  int usb_create_sysfs_dev_files(struct usb_device *udev)
 	struct device *dev = &udev->dev;
 	int retval;
 
-	retval = device_create_bin_file(dev, &dev_bin_attr_descriptors);
-	if (retval)
-		goto error;
-
 	retval = add_persist_attributes(dev);
 	if (retval)
 		goto error;
@@ -1071,7 +1116,6 @@  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_descriptors);
 }
 
 /* Interface Association Descriptor fields */