2019-04-23 07:24:17

by João Paulo Rechi Vita

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: Create new HCI_QUIRK_WAIT_FOR_MATCHING_CC

This commit creates a new quirk, HCI_QUIRK_WAIT_FOR_MATCHING_CC, which
when set makes the kernel not send the next queued HCI command until a
command complete arrives for the last HCI command sent to the
controller.

This avoids a state of confusion where the kernel believes a passive
scanning procedure is being performed while in fact controller is
performing an active scanning procedure, as requested by userspace and
the kernel.

This state of confusion is achieved when some buggy controllers send an
extra command complete event for the LE_SET_RANDOM_ADDR after the kernel
already sent the next queued command (LE_SET_SCAN_PARAM, for starting an
active scanning procedure), as shown at timestamp 27.420131 on the btmon
logs bellow.

Bluetooth monitor ver 5.50
= Note: Linux version 5.0.0+ (x86_64) 0.352340
= Note: Bluetooth subsystem version 2.22 0.352343
= New Index: 80:C5:F2:8F:87:84 (Primary,USB,hci0) [hci0] 0.352344
= Open Index: 80:C5:F2:8F:87:84 [hci0] 0.352345
= Index Info: 80:C5:F2:8F:87:84 (Qualcomm) [hci0] 0.352346
@ MGMT Open: bluetoothd (privileged) version 1.14 {0x0001} 0.352347
@ MGMT Open: btmon (privileged) version 1.14 {0x0002} 0.352366
@ MGMT Open: btmgmt (privileged) version 1.14 {0x0003} 27.302164
@ MGMT Command: Start Discovery (0x0023) plen 1 {0x0003} [hci0] 27.302310
Address type: 0x06
LE Public
LE Random
< HCI Command: LE Set Random Address (0x08|0x0005) plen 6 #1 [hci0] 27.302496
Address: 15:60:F2:91:B2:24 (Non-Resolvable)
> HCI Event: Command Complete (0x0e) plen 4 #2 [hci0] 27.419117
LE Set Random Address (0x08|0x0005) ncmd 1
Status: Success (0x00)
< HCI Command: LE Set Scan Parameters (0x08|0x000b) plen 7 #3 [hci0] 27.419244
Type: Active (0x01)
Interval: 11.250 msec (0x0012)
Window: 11.250 msec (0x0012)
Own address type: Random (0x01)
Filter policy: Accept all advertisement (0x00)
> HCI Event: Command Complete (0x0e) plen 4 #4 [hci0] 27.420131
LE Set Random Address (0x08|0x0005) ncmd 1
Status: Success (0x00)
< HCI Command: LE Set Scan Enable (0x08|0x000c) plen 2 #5 [hci0] 27.420259
Scanning: Enabled (0x01)
Filter duplicates: Enabled (0x01)
> HCI Event: Command Complete (0x0e) plen 4 #6 [hci0] 27.420969
LE Set Scan Parameters (0x08|0x000b) ncmd 1
Status: Success (0x00)
> HCI Event: Command Complete (0x0e) plen 4 #7 [hci0] 27.421983
LE Set Scan Enable (0x08|0x000c) ncmd 1
Status: Success (0x00)
@ MGMT Event: Command Complete (0x0001) plen 4 {0x0003} [hci0] 27.422059
Start Discovery (0x0023) plen 1
Status: Success (0x00)
Address type: 0x06
LE Public
LE Random
@ MGMT Event: Discovering (0x0013) plen 2 {0x0003} [hci0] 27.422067
Address type: 0x06
LE Public
LE Random
Discovery: Enabled (0x01)
@ MGMT Event: Discovering (0x0013) plen 2 {0x0002} [hci0] 27.422067
Address type: 0x06
LE Public
LE Random
Discovery: Enabled (0x01)
@ MGMT Event: Discovering (0x0013) plen 2 {0x0001} [hci0] 27.422067
Address type: 0x06
LE Public
LE Random
Discovery: Enabled (0x01)

In this situation the kernel ends up not processing the command complete
event for LE_SET_SCAN_PARAM, so hdev->le_scan_type is never updated to
active scanning and no device found events are generated for
ADV_SCAN_RSP PDUs. This makes impossible to discover BTLE devices with
these controllers.

Signed-off-by: João Paulo Rechi Vita <[email protected]>
---
include/net/bluetooth/hci.h | 4 ++++
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_core.c | 3 +++
net/bluetooth/hci_event.c | 4 ++++
4 files changed, 12 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index fbba43e9bef5..f100e9d566a0 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -204,6 +204,10 @@ enum {
*
*/
HCI_QUIRK_NON_PERSISTENT_SETUP,
+
+ /* When this quirk is set, no pending HCI commands will be sent until a
+ * matching command complete for the last command is received. */
+ HCI_QUIRK_WAIT_FOR_MATCHING_CC,
};

/* HCI device flags */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 094e61e07030..85bed4e916d3 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -364,6 +364,7 @@ struct hci_dev {
struct sk_buff_head cmd_q;

struct sk_buff *sent_cmd;
+ __u8 sent_cmd_pending_cc;

struct mutex req_lock;
wait_queue_head_t req_wait_q;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index d6b2540ba7f8..95ab8f9f2514 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -4383,6 +4383,8 @@ void hci_req_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status,
return;
}

+ hdev->sent_cmd_pending_cc = 0;
+
/* If the command succeeded and there's still more commands in
* this request the request is not yet complete.
*/
@@ -4493,6 +4495,7 @@ static void hci_cmd_work(struct work_struct *work)

hdev->sent_cmd = skb_clone(skb, GFP_KERNEL);
if (hdev->sent_cmd) {
+ hdev->sent_cmd_pending_cc = 1;
atomic_dec(&hdev->cmd_cnt);
hci_send_frame(hdev, skb);
if (test_bit(HCI_RESET, &hdev->flags))
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 609fd6871c5a..1abbca8ce1c2 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3404,6 +3404,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_req_cmd_complete(hdev, *opcode, *status, req_complete,
req_complete_skb);

+ if (hdev->sent_cmd_pending_cc &&
+ test_bit(HCI_QUIRK_WAIT_FOR_MATCHING_CC, &hdev->quirks))
+ return;
+
if (atomic_read(&hdev->cmd_cnt) && !skb_queue_empty(&hdev->cmd_q))
queue_work(hdev->workqueue, &hdev->cmd_work);
}
--
2.20.1