diff mbox series

[1/2] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

Message ID 20250121151532.1.If6f14aa2512336173a53fc3552756cd8a332b0a3@changeid
State Superseded
Headers show
Series [1/2] Bluetooth: Fix possible race with userspace of sysfs isoc_alt | expand

Commit Message

Hsin-chen Chuang Jan. 21, 2025, 7:15 a.m. UTC 
From: Hsin-chen Chuang <chharry@chromium.org>

Use device group to avoid the racing. To reuse the group defined in
hci_sysfs.c, defined 2 callbacks switch_usb_alt_setting and
read_usb_alt_setting which are only registered in btusb.

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

 drivers/bluetooth/btusb.c        | 42 ++++++++------------------------
 include/net/bluetooth/hci_core.h |  2 ++
 net/bluetooth/hci_sysfs.c        | 33 +++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 32 deletions(-)

Comments

Luiz Augusto von Dentz Jan. 21, 2025, 3:58 p.m. UTC | #1 
HI Hsin-chen,

On Tue, Jan 21, 2025 at 2:17 AM Hsin-chen Chuang <chharry@google.com> wrote:
>
> From: Hsin-chen Chuang <chharry@chromium.org>
>
> The SCO packets from Bluetooth raw socket are now rejected because
> hci_conn_num is left 0. This patch allows such the usecase to enable
> the userspace SCO support.
>
> Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
> ---
>
>  drivers/bluetooth/btusb.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index f419d0827ee4..3e2d7ea6de5c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2107,9 +2107,6 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
>                 return submit_or_queue_tx_urb(hdev, urb);
>
>         case HCI_SCODATA_PKT:
> -               if (hci_conn_num(hdev, SCO_LINK) < 1)
> -                       return -ENODEV;

Hmm, we probably need to check if USER_CHANNEL has been opened instead
of simply removing the check.

> -
>                 urb = alloc_isoc_urb(hdev, skb);
>                 if (IS_ERR(urb))
>                         return PTR_ERR(urb);
> @@ -2588,9 +2585,6 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
>                 return submit_or_queue_tx_urb(hdev, urb);
>
>         case HCI_SCODATA_PKT:
> -               if (hci_conn_num(hdev, SCO_LINK) < 1)
> -                       return -ENODEV;
> -
>                 urb = alloc_isoc_urb(hdev, skb);
>                 if (IS_ERR(urb))
>                         return PTR_ERR(urb);
> --
> 2.48.0.rc2.279.g1de40edade-goog
>
diff mbox series

Patch

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 9aa018d4f6f5..f419d0827ee4 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2224,6 +2224,13 @@  static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts)
 	return 0;
 }
 
+static int btusb_read_alt_setting(struct hci_dev *hdev)
+{
+	struct btusb_data *data = hci_get_drvdata(hdev);
+
+	return data->isoc_altsetting;
+}
+
 static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
 							int alt)
 {
@@ -3646,32 +3653,6 @@  static const struct file_operations force_poll_sync_fops = {
 	.llseek		= default_llseek,
 };
 
-static ssize_t isoc_alt_show(struct device *dev,
-			     struct device_attribute *attr,
-			     char *buf)
-{
-	struct btusb_data *data = dev_get_drvdata(dev);
-
-	return sysfs_emit(buf, "%d\n", data->isoc_altsetting);
-}
-
-static ssize_t isoc_alt_store(struct device *dev,
-			      struct device_attribute *attr,
-			      const char *buf, size_t count)
-{
-	struct btusb_data *data = dev_get_drvdata(dev);
-	int alt;
-	int ret;
-
-	if (kstrtoint(buf, 10, &alt))
-		return -EINVAL;
-
-	ret = btusb_switch_alt_setting(data->hdev, alt);
-	return ret < 0 ? ret : count;
-}
-
-static DEVICE_ATTR_RW(isoc_alt);
-
 static int btusb_probe(struct usb_interface *intf,
 		       const struct usb_device_id *id)
 {
@@ -4036,9 +4017,8 @@  static int btusb_probe(struct usb_interface *intf,
 		if (err < 0)
 			goto out_free_dev;
 
-		err = device_create_file(&intf->dev, &dev_attr_isoc_alt);
-		if (err)
-			goto out_free_dev;
+		hdev->switch_usb_alt_setting = btusb_switch_alt_setting;
+		hdev->read_usb_alt_setting = btusb_read_alt_setting;
 	}
 
 	if (IS_ENABLED(CONFIG_BT_HCIBTUSB_BCM) && data->diag) {
@@ -4085,10 +4065,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);
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f756fac95488..5d3ec5ff5adb 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -641,6 +641,8 @@  struct hci_dev {
 				     struct bt_codec *codec, __u8 *vnd_len,
 				     __u8 **vnd_data);
 	u8 (*classify_pkt_type)(struct hci_dev *hdev, struct sk_buff *skb);
+	int (*switch_usb_alt_setting)(struct hci_dev *hdev, int new_alts);
+	int (*read_usb_alt_setting)(struct hci_dev *hdev);
 };
 
 #define HCI_PHY_HANDLE(handle)	(handle & 0xff)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 041ce9adc378..887aa1935b1e 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -102,8 +102,41 @@  static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_WO(reset);
 
+static ssize_t isoc_alt_show(struct device *dev,
+			     struct device_attribute *attr,
+			     char *buf)
+{
+	struct hci_dev *hdev = to_hci_dev(dev);
+
+	if (hdev->read_usb_alt_setting)
+		return sysfs_emit(buf, "%d\n", hdev->read_usb_alt_setting(hdev));
+
+	return -ENODEV;
+}
+
+static ssize_t isoc_alt_store(struct device *dev,
+			      struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct hci_dev *hdev = to_hci_dev(dev);
+	int alt;
+	int ret;
+
+	if (kstrtoint(buf, 10, &alt))
+		return -EINVAL;
+
+	if (hdev->switch_usb_alt_setting) {
+		ret = hdev->switch_usb_alt_setting(hdev, alt);
+		return ret < 0 ? ret : count;
+	}
+
+	return -ENODEV;
+}
+static DEVICE_ATTR_RW(isoc_alt);
+
 static struct attribute *bt_host_attrs[] = {
 	&dev_attr_reset.attr,
+	&dev_attr_isoc_alt.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(bt_host);