Message ID | 20191017032039.18413-1-mkorpershoek@baylibre.com |
---|---|
State | Awaiting Upstream |
Delegated to: | David Miller |
Headers | show |
Series | [v2] Bluetooth: hci_core: fix init for HCI_USER_CHANNEL | expand |
Hi Mattijs, > During the setup() stage, HCI device drivers expect the chip to > acknowledge its setup() completion via vendor specific frames. > > If userspace opens() such HCI device in HCI_USER_CHANNEL [1] mode, > the vendor specific frames are never tranmitted to the driver, as > they are filtered in hci_rx_work(). > > Allow HCI devices which operate in HCI_USER_CHANNEL mode to receive > frames if the HCI device is is HCI_INIT state. > > [1] https://www.spinics.net/lists/linux-bluetooth/msg37345.html > > Fixes: 23500189d7e0 ("Bluetooth: Introduce new HCI socket channel for user operation") > Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > --- > Changelog: > v2: > * change test logic to transfer packets when in INIT phase > for user channel mode as recommended by Marcel > * renamed patch from > "Bluetooth: hci_core: fix init with HCI_QUIRK_NON_PERSISTENT_SETUP" > > v1: > * https://lkml.org/lkml/2019/10/3/2250 > > Some more background on the change follows: > > The Android bluetooth stack (Bluedroid) also has a HAL implementation > which follows Linux's standard rfkill interface [1]. > > This implementation relies on the HCI_CHANNEL_USER feature to get > exclusive access to the underlying bluetooth device. > > When testing this along with the btkmtksdio driver, the > chip appeared unresponsive when calling the following from userspace: > > struct sockaddr_hci addr; > int fd; > > fd = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI); > > memset(&addr, 0, sizeof(addr)); > addr.hci_family = AF_BLUETOOTH; > addr.hci_dev = 0; > addr.hci_channel = HCI_CHANNEL_USER; > > bind(fd, (struct sockaddr *) &addr, sizeof(addr)); # device hangs > > In the case of bluetooth drivers exposing QUIRK_NON_PERSISTENT_SETUP > such as btmtksdio, setup() is called each multiple times. > In particular, when userspace calls bind(), the setup() is called again > and vendor specific commands might be send to re-initialize the chip. > > Those commands are filtered out by hci_core in HCI_CHANNEL_USER mode, > preventing setup() from completing successfully. > > This has been tested on a 4.19 kernel based on Android Common Kernel. > It has also been compile tested on bluetooth-next. > > [1] https://android.googlesource.com/platform/system/bt/+/refs/heads/master/vendor_libs/linux/interface/ > > net/bluetooth/hci_core.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) patch has been applied to bluetooth-next tree. Regards Marcel
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index b2559d4bed81..0cc9ce917222 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -4440,7 +4440,14 @@ static void hci_rx_work(struct work_struct *work) hci_send_to_sock(hdev, skb); } - if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { + /* If the device has been opened in HCI_USER_CHANNEL, + * the userspace has exclusive access to device. + * When device is HCI_INIT, we still need to process + * the data packets to the driver in order + * to complete its setup(). + */ + if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && + !test_bit(HCI_INIT, &hdev->flags)) { kfree_skb(skb); continue; }
During the setup() stage, HCI device drivers expect the chip to acknowledge its setup() completion via vendor specific frames. If userspace opens() such HCI device in HCI_USER_CHANNEL [1] mode, the vendor specific frames are never tranmitted to the driver, as they are filtered in hci_rx_work(). Allow HCI devices which operate in HCI_USER_CHANNEL mode to receive frames if the HCI device is is HCI_INIT state. [1] https://www.spinics.net/lists/linux-bluetooth/msg37345.html Fixes: 23500189d7e0 ("Bluetooth: Introduce new HCI socket channel for user operation") Signed-off-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> --- Changelog: v2: * change test logic to transfer packets when in INIT phase for user channel mode as recommended by Marcel * renamed patch from "Bluetooth: hci_core: fix init with HCI_QUIRK_NON_PERSISTENT_SETUP" v1: * https://lkml.org/lkml/2019/10/3/2250 Some more background on the change follows: The Android bluetooth stack (Bluedroid) also has a HAL implementation which follows Linux's standard rfkill interface [1]. This implementation relies on the HCI_CHANNEL_USER feature to get exclusive access to the underlying bluetooth device. When testing this along with the btkmtksdio driver, the chip appeared unresponsive when calling the following from userspace: struct sockaddr_hci addr; int fd; fd = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI); memset(&addr, 0, sizeof(addr)); addr.hci_family = AF_BLUETOOTH; addr.hci_dev = 0; addr.hci_channel = HCI_CHANNEL_USER; bind(fd, (struct sockaddr *) &addr, sizeof(addr)); # device hangs In the case of bluetooth drivers exposing QUIRK_NON_PERSISTENT_SETUP such as btmtksdio, setup() is called each multiple times. In particular, when userspace calls bind(), the setup() is called again and vendor specific commands might be send to re-initialize the chip. Those commands are filtered out by hci_core in HCI_CHANNEL_USER mode, preventing setup() from completing successfully. This has been tested on a 4.19 kernel based on Android Common Kernel. It has also been compile tested on bluetooth-next. [1] https://android.googlesource.com/platform/system/bt/+/refs/heads/master/vendor_libs/linux/interface/ net/bluetooth/hci_core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)