linux-kernel - Re: [PATCH v3] Bluetooth: btusb: Configure altsetting for HCI_USER

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CADg1FFcK1e_y6mroLuh2kkWA32BU3esbtT9vLbntviDUtkoLJw@mail.gmail.com>
Date: Fri, 28 Feb 2025 09:08:29 +0800
From: Hsin-chen Chuang <chharry@...gle.com>
To: Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc: linux-bluetooth@...r.kernel.org, gregkh@...uxfoundation.org, 
	pmenzel@...gen.mpg.de, chromeos-bluetooth-upstreaming@...omium.org, 
	Hsin-chen Chuang <chharry@...omium.org>, Marcel Holtmann <marcel@...tmann.org>, 
	Ying Hsu <yinghsu@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] Bluetooth: btusb: Configure altsetting for HCI_USER_CHANNEL

Hi Luiz,

On Fri, Feb 28, 2025 at 5:40 AM Luiz Augusto von Dentz
<luiz.dentz@...il.com> wrote:
>
> Hi Hsin-chen,
>
> On Thu, Feb 27, 2025 at 12:14 PM Hsin-chen Chuang <chharry@...gle.com> wrote:
> >
> > From: Hsin-chen Chuang <chharry@...omium.org>
> >
> > Automatically configure the altsetting for HCI_USER_CHANNEL when a SCO
> > is connected.
> >
> > The motivation is to enable the HCI_USER_CHANNEL user to send out SCO
> > data through USB Bluetooth chips, which is mainly used for bidirectional
> > audio transfer (voice call). This was not capable because:
> >
> > - Per Bluetooth Core Spec v5, Vol 4, Part B, 2.1, the corresponding
> >   alternate setting should be set based on the air mode in order to
> >   transfer SCO data, but
> > - The Linux Bluetooth HCI_USER_CHANNEL exposes the Bluetooth Host
> >   Controller Interface to the user space, which is something above the
> >   USB layer. The user space is not able to configure the USB alt while
> >   keeping the channel open.
> >
> > This patch intercepts the HCI_EV_SYNC_CONN_COMPLETE packets in btusb,
> > extracts the air mode, and configures the alt setting in btusb.
> >
> > This patch is tested on ChromeOS devices. The USB Bluetooth models
> > (CVSD, TRANS alt3 and alt6) could work without a customized kernel.
> >
> > Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> > Signed-off-by: Hsin-chen Chuang <chharry@...omium.org>
> > ---
> >
> > Changes in v3:
> > - Remove module parameter
> > - Set Kconfig to default y if CHROME_PLATFORMS
> >
> > Changes in v2:
> > - Give up tracking the SCO handles. Only configure the altsetting when
> >   SCO connected.
> > - Put the change behind Kconfig/module parameter
> >
> >  drivers/bluetooth/Kconfig | 11 ++++++++++
> >  drivers/bluetooth/btusb.c | 43 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 54 insertions(+)
> >
> > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > index 4ab32abf0f48..cdf7a5caa5c8 100644
> > --- a/drivers/bluetooth/Kconfig
> > +++ b/drivers/bluetooth/Kconfig
> > @@ -56,6 +56,17 @@ config BT_HCIBTUSB_POLL_SYNC
> >           Say Y here to enable USB poll_sync for Bluetooth USB devices by
> >           default.
> >
> > +config BT_HCIBTUSB_AUTO_SET_ISOC_ALT
> > +       bool "Auto set isoc_altsetting for HCI_USER_CHANNEL when SCO connected"
> > +       depends on BT_HCIBTUSB
> > +       default y if CHROME_PLATFORMS
> > +       help
> > +         Say Y here to enable auto set isoc_altsetting for HCI_USER_CHANNEL
> > +         when SCO connected
> > +
> > +         When enabled, btusb intercepts the HCI_EV_SYNC_CONN_COMPLETE packets
> > +         and configures isoc_altsetting automatically for HCI_USER_CHANNEL.
> > +
> >  config BT_HCIBTUSB_BCM
> >         bool "Broadcom protocol support"
> >         depends on BT_HCIBTUSB
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index de3fa725d210..2642d2ca885f 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -34,6 +34,8 @@ static bool force_scofix;
> >  static bool enable_autosuspend = IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTOSUSPEND);
> >  static bool enable_poll_sync = IS_ENABLED(CONFIG_BT_HCIBTUSB_POLL_SYNC);
> >  static bool reset = true;
> > +static bool auto_set_isoc_alt =
> > +       IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTO_SET_ISOC_ALT);
> >
> >  static struct usb_driver btusb_driver;
> >
> > @@ -1113,6 +1115,42 @@ static inline void btusb_free_frags(struct btusb_data *data)
> >         spin_unlock_irqrestore(&data->rxlock, flags);
> >  }
> >
> > +static void btusb_sco_connected(struct btusb_data *data, struct sk_buff *skb)
> > +{
> > +       struct hci_event_hdr *hdr = (void *) skb->data;
> > +       struct hci_ev_sync_conn_complete *ev =
> > +               (void *) skb->data + sizeof(*hdr);
> > +       struct hci_dev *hdev = data->hdev;
> > +       unsigned int notify_air_mode;
> > +
> > +       if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT)
> > +               return;
> > +
> > +       if (skb->len < sizeof(*hdr) || hdr->evt != HCI_EV_SYNC_CONN_COMPLETE)
> > +               return;
> > +
> > +       if (skb->len != sizeof(*hdr) + sizeof(*ev) || ev->status)
> > +               return;
> > +
> > +       switch (ev->air_mode) {
> > +       case BT_CODEC_CVSD:
> > +               notify_air_mode = HCI_NOTIFY_ENABLE_SCO_CVSD;
> > +               break;
> > +
> > +       case BT_CODEC_TRANSPARENT:
> > +               notify_air_mode = HCI_NOTIFY_ENABLE_SCO_TRANSP;
> > +               break;
> > +
> > +       default:
> > +               return;
> > +       }
> > +
> > +       bt_dev_info(hdev, "enabling SCO with air mode %u", ev->air_mode);
> > +       data->sco_num = 1;
> > +       data->air_mode = notify_air_mode;
> > +       schedule_work(&data->work);
> > +}
> > +
> >  static int btusb_recv_event(struct btusb_data *data, struct sk_buff *skb)
> >  {
> >         if (data->intr_interval) {
> > @@ -1120,6 +1158,11 @@ static int btusb_recv_event(struct btusb_data *data, struct sk_buff *skb)
> >                 schedule_delayed_work(&data->rx_work, 0);
> >         }
> >
> > +       /* Configure altsetting for HCI_USER_CHANNEL on SCO connected */
> > +       if (auto_set_isoc_alt &&
> > +           hci_dev_test_flag(data->hdev, HCI_USER_CHANNEL))
> > +               btusb_sco_connected(data, skb);
> > +
> >         return data->recv_event(data->hdev, skb);
> >  }
> >
> > --
> > 2.48.1.658.g4767266eb4-goog
>
> This has been merged, I did some minor rewording here and there but
> the logic remains the same, now we can problem revert b16b327edb4d
> ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")?
>
> --
> Luiz Augusto von Dentz

Yes, sure. We could revert b16b327edb4d now.


Best Regards,

Hsin-chen