From 136a73da8e3d771567a0a78e0af867d3f9cf63eb Mon Sep 17 00:00:00 2001 From: Guan-Yu Lin Date: Fri, 2 Feb 2024 03:00:42 +0000 Subject: usb: sysfs: use kstrtobool() if possible Replace the self-rolled implementations with kstrtobool(). This reduces the maintenance efforts in the future. Signed-off-by: Guan-Yu Lin Link: https://lore.kernel.org/r/20240202030301.2396374-1-guanyulin@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/sysfs.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'drivers/usb/core/sysfs.c') diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index 5d21718afb05..e67826ec053e 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -273,9 +273,10 @@ static ssize_t avoid_reset_quirk_store(struct device *dev, const char *buf, size_t count) { struct usb_device *udev = to_usb_device(dev); - int val, rc; + bool val; + int rc; - if (sscanf(buf, "%d", &val) != 1 || val < 0 || val > 1) + if (kstrtobool(buf, &val) != 0) return -EINVAL; rc = usb_lock_device_interruptible(udev); if (rc < 0) @@ -322,13 +323,14 @@ static ssize_t persist_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct usb_device *udev = to_usb_device(dev); - int value, rc; + bool value; + int rc; /* Hubs are always enabled for USB_PERSIST */ if (udev->descriptor.bDeviceClass == USB_CLASS_HUB) return -EPERM; - if (sscanf(buf, "%d", &value) != 1) + if (kstrtobool(buf, &value) != 0) return -EINVAL; rc = usb_lock_device_interruptible(udev); @@ -739,14 +741,14 @@ static ssize_t authorized_store(struct device *dev, { ssize_t result; struct usb_device *usb_dev = to_usb_device(dev); - unsigned val; - result = sscanf(buf, "%u\n", &val); - if (result != 1) + bool val; + + if (kstrtobool(buf, &val) != 0) result = -EINVAL; - else if (val == 0) - result = usb_deauthorize_device(usb_dev); - else + else if (val) result = usb_authorize_device(usb_dev); + else + result = usb_deauthorize_device(usb_dev); return result < 0 ? result : size; } static DEVICE_ATTR_IGNORE_LOCKDEP(authorized, S_IRUGO | S_IWUSR, -- cgit v1.2.3 From 12fc84e8c4288cc8ed5f14a35e077130c2cfece2 Mon Sep 17 00:00:00 2001 From: Elbert Mai Date: Mon, 4 Mar 2024 16:23:01 -0800 Subject: usb: Export BOS descriptor to sysfs 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 Link: https://lore.kernel.org/r/20240305002301.95323-1-code@elbertmai.com Signed-off-by: Greg Kroah-Hartman --- Documentation/ABI/testing/sysfs-bus-usb | 10 +++++ drivers/usb/core/sysfs.c | 78 ++++++++++++++++++++++++++------- 2 files changed, 71 insertions(+), 17 deletions(-) (limited to 'drivers/usb/core/sysfs.c') diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb index 2b7108e21977..af9b653422f1 100644 --- a/Documentation/ABI/testing/sysfs-bus-usb +++ b/Documentation/ABI/testing/sysfs-bus-usb @@ -442,6 +442,16 @@ What: /sys/bus/usb/devices/usbX/descriptors Description: Contains the interface descriptors, in binary. +What: /sys/bus/usb/devices/usbX/bos_descriptors +Date: March 2024 +Contact: Elbert Mai +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. + What: /sys/bus/usb/devices/usbX/idProduct Description: Product ID, in hexadecimal. diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index e67826ec053e..777526f59720 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -849,16 +849,10 @@ static const 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) { @@ -880,7 +874,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) { @@ -895,11 +889,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 }; /* @@ -1017,10 +1066,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; @@ -1050,7 +1095,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 */ -- cgit v1.2.3 From dfea18989aa7beb42c2cb6344fe8787de35d9471 Mon Sep 17 00:00:00 2001 From: Elbert Mai Date: Tue, 5 Mar 2024 16:15:03 -0800 Subject: usb: Clarify expected behavior of dev_bin_attrs_are_visible() The commit "usb: Export BOS descriptor to sysfs" added a binary attribute group to sysfs. It doesn't check if the descriptors attribute should be visible, which is by design and not an oversight. Update a comment so that it better explains this in the dev_bin_attrs_are_visible() function. Signed-off-by: Elbert Mai Link: https://lore.kernel.org/r/20240306001503.313028-1-code@elbertmai.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/sysfs.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/usb/core/sysfs.c') diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index 777526f59720..f98263e21c2a 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -929,8 +929,11 @@ static umode_t dev_bin_attrs_are_visible(struct kobject *kobj, 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. + /* + * There's no need to check if the descriptors attribute should + * be visible because all devices have a device descriptor. The + * bos_descriptors attribute should be visible if and only if + * the device has a BOS, so check if it exists here. */ if (a == &bin_attr_bos_descriptors) { if (udev->bos == NULL) -- cgit v1.2.3 From 80ba43e9f799cbdd83842fc27db667289b3150f5 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Tue, 12 Mar 2024 11:48:23 -0400 Subject: USB: core: Fix deadlock in usb_deauthorize_interface() Among the attribute file callback routines in drivers/usb/core/sysfs.c, the interface_authorized_store() function is the only one which acquires a device lock on an ancestor device: It calls usb_deauthorize_interface(), which locks the interface's parent USB device. The will lead to deadlock if another process already owns that lock and tries to remove the interface, whether through a configuration change or because the device has been disconnected. As part of the removal procedure, device_del() waits for all ongoing sysfs attribute callbacks to complete. But usb_deauthorize_interface() can't complete until the device lock has been released, and the lock won't be released until the removal has finished. The mechanism provided by sysfs to prevent this kind of deadlock is to use the sysfs_break_active_protection() function, which tells sysfs not to wait for the attribute callback. Reported-and-tested by: Yue Sun Reported by: xingwei lee Signed-off-by: Alan Stern Link: https://lore.kernel.org/linux-usb/CAEkJfYO6jRVC8Tfrd_R=cjO0hguhrV31fDPrLrNOOHocDkPoAA@mail.gmail.com/#r Fixes: 310d2b4124c0 ("usb: interface authorization: SysFS part of USB interface authorization") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/1c37eea1-9f56-4534-b9d8-b443438dc869@rowland.harvard.edu Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/sysfs.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'drivers/usb/core/sysfs.c') diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index f98263e21c2a..d83231d6736a 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -1217,14 +1217,24 @@ static ssize_t interface_authorized_store(struct device *dev, { struct usb_interface *intf = to_usb_interface(dev); bool val; + struct kernfs_node *kn; if (kstrtobool(buf, &val) != 0) return -EINVAL; - if (val) + if (val) { usb_authorize_interface(intf); - else - usb_deauthorize_interface(intf); + } else { + /* + * Prevent deadlock if another process is concurrently + * trying to unregister intf. + */ + kn = sysfs_break_active_protection(&dev->kobj, &attr->attr); + if (kn) { + usb_deauthorize_interface(intf); + sysfs_unbreak_active_protection(kn); + } + } return count; } -- cgit v1.2.3