[PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

Hsin-chen Chuang
2025-02-13T03:44:10Z

From: Hsin-chen Chuang <chharry@chromium.org>

Expose the isoc_alt attr with device group to avoid the racing.

Now we create a dev node for btusb. The isoc_alt attr belongs to it and
it also becomes the parent device of hci dev.

Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---

Changes in v4:
- Create a dev node for btusb. It's now hci dev's parent and the
isoc_alt now belongs to it.
- Since the changes is almost limitted in btusb, no need to add the
callbacks in hdev anymore.

Changes in v3:
- Make the attribute exported only when the isoc_alt is available.
- In btusb_probe, determine data->isoc before calling hci_alloc_dev_priv
(which calls hci_init_sysfs).
- Since hci_init_sysfs is called before btusb could modify the hdev,
add new argument add_isoc_alt_attr for btusb to inform hci_init_sysfs.

drivers/bluetooth/btusb.c | 98 +++++++++++++++++++++++++-------
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_sysfs.c | 3 +-
3 files changed, 79 insertions(+), 23 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 1caf7a071a73..cb3db18bb72c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -920,6 +920,8 @@ struct btusb_data {
int oob_wake_irq; /* irq for out-of-band wake-on-bt */

struct qca_dump_info qca_dump;
+
+ struct device dev;
};

static void btusb_reset(struct hci_dev *hdev)
@@ -3693,6 +3695,9 @@ static ssize_t isoc_alt_store(struct device *dev,
int alt;
int ret;

+ if (!data->hdev)
+ return -ENODEV;
+
if (kstrtoint(buf, 10, &alt))
return -EINVAL;

@@ -3702,6 +3707,34 @@ static ssize_t isoc_alt_store(struct device *dev,

static DEVICE_ATTR_RW(isoc_alt);

+static struct attribute *btusb_sysfs_attrs[] = {
+ NULL,
+};
+ATTRIBUTE_GROUPS(btusb_sysfs);
+
+static void btusb_sysfs_release(struct device *dev)
+{
+ // Resource release is managed in btusb_disconnect
+}
+
+static const struct device_type btusb_sysfs = {
+ .name = "btusb",
+ .release = btusb_sysfs_release,
+ .groups = btusb_sysfs_groups,
+};
+
+static struct attribute *btusb_sysfs_isoc_alt_attrs[] = {
+ &dev_attr_isoc_alt.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(btusb_sysfs_isoc_alt);
+
+static const struct device_type btusb_sysfs_isoc_alt = {
+ .name = "btusb",
+ .release = btusb_sysfs_release,
+ .groups = btusb_sysfs_isoc_alt_groups,
+};
+
static int btusb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
@@ -3821,16 +3854,47 @@ static int btusb_probe(struct usb_interface *intf,

data->recv_acl = hci_recv_frame;

+ if (id->driver_info & BTUSB_AMP) {
+ /* AMP controllers do not support SCO packets */
+ data->isoc = NULL;
+ } else {
+ /* Interface orders are hardcoded in the specification */
+ data->isoc = usb_ifnum_to_if(data->udev, ifnum_base + 1);
+ data->isoc_ifnum = ifnum_base + 1;
+ }
+
+ if (id->driver_info & BTUSB_BROKEN_ISOC)
+ data->isoc = NULL;
+
+ /* Init a dev for btusb. The attr depends on the support of isoc. */
+ if (data->isoc)
+ data->dev.type = &btusb_sysfs_isoc_alt;
+ else
+ data->dev.type = &btusb_sysfs;
+ data->dev.class = &bt_class;
+ data->dev.parent = &intf->dev;
+
+ err = dev_set_name(&data->dev, "btusb%s", dev_name(&intf->dev));
+ if (err)
+ return err;
+
+ dev_set_drvdata(&data->dev, data);
+ err = device_register(&data->dev);
+ if (err < 0)
+ goto out_put_sysfs;
+
hdev = hci_alloc_dev_priv(priv_size);
- if (!hdev)
- return -ENOMEM;
+ if (!hdev) {
+ err = -ENOMEM;
+ goto out_free_sysfs;
+ }

hdev->bus = HCI_USB;
hci_set_drvdata(hdev, data);

data->hdev = hdev;

- SET_HCIDEV_DEV(hdev, &intf->dev);
+ SET_HCIDEV_DEV(hdev, &data->dev);

reset_gpio = gpiod_get_optional(&data->udev->dev, "reset",
GPIOD_OUT_LOW);
@@ -3969,15 +4033,6 @@ static int btusb_probe(struct usb_interface *intf,
hci_set_msft_opcode(hdev, 0xFD70);
}

- if (id->driver_info & BTUSB_AMP) {
- /* AMP controllers do not support SCO packets */
- data->isoc = NULL;
- } else {
- /* Interface orders are hardcoded in the specification */
- data->isoc = usb_ifnum_to_if(data->udev, ifnum_base + 1);
- data->isoc_ifnum = ifnum_base + 1;
- }
-
if (IS_ENABLED(CONFIG_BT_HCIBTUSB_RTL) &&
(id->driver_info & BTUSB_REALTEK)) {
btrtl_set_driver_name(hdev, btusb_driver.name);
@@ -4010,9 +4065,6 @@ static int btusb_probe(struct usb_interface *intf,
set_bit(HCI_QUIRK_FIXUP_BUFFER_SIZE, &hdev->quirks);
}

- if (id->driver_info & BTUSB_BROKEN_ISOC)
- data->isoc = NULL;
-
if (id->driver_info & BTUSB_WIDEBAND_SPEECH)
set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks);

@@ -4065,10 +4117,6 @@ static int btusb_probe(struct usb_interface *intf,
data->isoc, data);
if (err < 0)
goto out_free_dev;
-
- err = device_create_file(&intf->dev, &dev_attr_isoc_alt);
- if (err)
- goto out_free_dev;
}

if (IS_ENABLED(CONFIG_BT_HCIBTUSB_BCM) && data->diag) {
@@ -4099,6 +4147,13 @@ static int btusb_probe(struct usb_interface *intf,
if (data->reset_gpio)
gpiod_put(data->reset_gpio);
hci_free_dev(hdev);
+
+out_free_sysfs:
+ device_del(&data->dev);
+
+out_put_sysfs:
+ put_device(&data->dev);
+
return err;
}

@@ -4115,10 +4170,8 @@ static void btusb_disconnect(struct usb_interface *intf)
hdev = data->hdev;
usb_set_intfdata(data->intf, NULL);

- if (data->isoc) {
- device_remove_file(&intf->dev, &dev_attr_isoc_alt);
+ if (data->isoc)
usb_set_intfdata(data->isoc, NULL);
- }

if (data->diag)
usb_set_intfdata(data->diag, NULL);
@@ -4150,6 +4203,7 @@ static void btusb_disconnect(struct usb_interface *intf)
gpiod_put(data->reset_gpio);

hci_free_dev(hdev);
+ device_unregister(&data->dev);
}

#ifdef CONFIG_PM
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 05919848ea95..776dd6183509 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1843,6 +1843,7 @@ int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);

void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);

+extern const struct class bt_class;
void hci_init_sysfs(struct hci_dev *hdev);
void hci_conn_init_sysfs(struct hci_conn *conn);
void hci_conn_add_sysfs(struct hci_conn *conn);
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 041ce9adc378..aab3ffaa264c 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -6,9 +6,10 @@
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>

-static const struct class bt_class = {
+const struct class bt_class = {
.name = "bluetooth",
};
+EXPORT_SYMBOL(bt_class);

static void bt_link_release(struct device *dev)
{
--
2.48.1.502.g6dc24dfdaf-goog