Message-ID: <CADg1FFd3nWiZqA8huodnsjezgrAL-p9t2BLHf3MzO3cJD6xZ+w@mail.gmail.com>
Date: Mon, 17 Feb 2025 17:17:50 +0800
From: Hsin-chen Chuang <chharry@...gle.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: linux-bluetooth@...r.kernel.org, luiz.dentz@...il.com, 
	chromeos-bluetooth-upstreaming@...omium.org, 
	Hsin-chen Chuang <chharry@...omium.org>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
	Johan Hedberg <johan.hedberg@...il.com>, Marcel Holtmann <marcel@...tmann.org>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Ying Hsu <yinghsu@...omium.org>, 
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v5] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

On Mon, Feb 17, 2025 at 4:53 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Mon, Feb 17, 2025 at 04:44:35PM +0800, Hsin-chen Chuang wrote:
> > On Fri, Feb 14, 2025 at 7:37 PM Greg KH <gregkh@...uxfoundation.org> wrote:
> > >
> > > On Fri, Feb 14, 2025 at 07:16:17PM +0800, Hsin-chen Chuang wrote:
> > > > From: Hsin-chen Chuang <chharry@...omium.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")
> > >
> > > Wait, step back, why is this commit needed if you can change the alt
> > > setting already today through usbfs/libusb without needing to mess with
> > > the bluetooth stack at all?
> >
> > In short: We want to configure the alternate settings without
> > detaching the btusb driver, while detaching seems necessary for
> > libusb_set_interface_alt_setting to work (Please correct me if I'm
> > wrong!)
>
> I think changing the alternate setting should work using usbfs as you
> would send that command to the device, not the interface, so the driver
> bound to the existing interface would not need to be removed.
>
> Try it out and see yourself to verify this before you continue down any
> of this.  There's no need to use libfs for just a single usbfs command,
> right?

I will give it a try. Great thanks for this suggestion!

>
> thanks,
>
> greg k-h


On Mon, Feb 17, 2025 at 4:55 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Mon, Feb 17, 2025 at 04:44:35PM +0800, Hsin-chen Chuang wrote:
> > On Fri, Feb 14, 2025 at 7:37 PM Greg KH <gregkh@...uxfoundation.org> wrote:
> > >
> > > On Fri, Feb 14, 2025 at 07:16:17PM +0800, Hsin-chen Chuang wrote:
> > > > From: Hsin-chen Chuang <chharry@...omium.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")
> > >
> > > Wait, step back, why is this commit needed if you can change the alt
> > > setting already today through usbfs/libusb without needing to mess with
> > > the bluetooth stack at all?
> >
> > In short: We want to configure the alternate settings without
> > detaching the btusb driver, while detaching seems necessary for
> > libusb_set_interface_alt_setting to work (Please correct me if I'm
> > wrong!)
> >
> > Background:
> > The Bluetooth Core Specification defines a protocol for the operating
> > system to communicate with a Bluetooth chipset, called HCI (Host
> > Controller Interface) (Host=OS, Controller=chipset).
> > We could say the main purpose of the Linux Bluetooth drivers is to set
> > up and get the HCI ready for the "upper layer" to use.
> >
> > Who could be the "upper layer" then? There are mainly 2: "Control" and
> > "User" channels.
> > Linux has its default Bluetooth stack, BlueZ, which is splitted into 2
> > parts: the kernel space and the user space. The kernel space part
> > provides an abstracted Bluetooth API called MGMT, and is exposed
> > through the Bluetooth HCI socket "Control" channel.
> > On the other hand Linux also exposes the Bluetooth HCI socket "User"
> > channel, allowing the user space APPs to send/receive the HCI packets
> > directly to/from the chipset. Google's products (Android, ChromeOS,
> > etc) use this channel.
> >
> > Now why this patch?
> > It's because the Bluetooth spec defines something specific to USB
> > transport: A USB Bluetooth chipset must/should support these alternate
> > settings; When transferring this type of the Audio data this alt must
> > be used, bla bla bla...
> > The Control channel handles this in the kernel part. However, the
> > applications built on top of the User channel are unable to configure
> > the alt setting, and I'd like to add the support through sysfs.
>
> So the "problem" is that Google doesn't want to use BlueZ, which is
> fine, you do you :)
>
> But how does BlueZ handle this same problem today?  What api to the
> kernel does it use to change the interface that you can't also do with
> your "BlueZ replacement"?
>
> Surely this isn't a new issue suddenly, but if it is, it needs to be
> solved so BOTH userspace stacks can handle it.

BlueZ handles that in their MGMT command, that is, through Control
channel -> BlueZ kernel space code -> driver callbacks.
Once a Bluetooth chipset is opened with the User channel, it can't be
used with the Control channel simultaneously, and vice versa.

>
> thanks,
>
> greg k-h

-- 
Best Regards,
Hsin-chen

Powered by blists - more mailing lists