From f6fc86f2c572ff1d192e8b5d5bf339ba06ebe3e4 Mon Sep 17 00:00:00 2001 From: Kuba Pawlak Date: Wed, 28 Oct 2015 15:18:05 +0100 Subject: Bluetooth: Fix possible deadlock in btusb commit 8f9d02f470f48416444ac3a1eacecdd0f743f1a7 introduced spinlocks in btusb_work. This is run in a context of a worqueue and can be interrupted by hardware irq. If it happens while spinlock is held, we have a deadlock. Solution is to use _irqsave/_resore version of locking [ 466.460560] ================================= [ 466.460565] [ INFO: inconsistent lock state ] [ 466.460572] 4.3.0-rc6+ #1 Tainted: G W [ 466.460576] --------------------------------- [ 466.460582] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage. [ 466.460589] kworker/0:2/94 [HC0[0]:SC0[0]:HE1:SE1] takes: [ 466.460595] (&(&data->rxlock)->rlock){?.-...}, at: [] btusb_work+0xa3/0x3fd [btusb] [ 466.460621] {IN-HARDIRQ-W} state was registered at: [ 466.460625] [] __lock_acquire+0xc45/0x1e80 [ 466.460638] [] lock_acquire+0xe5/0x1f0 [ 466.460646] [] _raw_spin_lock+0x38/0x50 [ 466.460657] [] btusb_recv_intr+0x38/0x170 [btusb] [ 466.460668] [] btusb_intr_complete+0xa6/0x130 [btusb] [ 466.460679] [] __usb_hcd_giveback_urb+0x8e/0x160 [ 466.460690] [] usb_hcd_giveback_urb+0x3f/0x120 [ 466.460698] [] uhci_giveback_urb+0xad/0x280 [ 466.460706] [] uhci_scan_schedule.part.33+0x6b4/0xbe0 [ 466.460714] [] uhci_irq+0xd0/0x180 [ 466.460722] [] usb_hcd_irq+0x26/0x40 [ 466.460729] [] handle_irq_event_percpu+0x40/0x300 [ 466.460739] [] handle_irq_event+0x40/0x60 [ 466.460746] [] handle_fasteoi_irq+0x89/0x150 [ 466.460754] [] handle_irq+0x73/0x120 [ 466.460763] [] do_IRQ+0x61/0x120 [ 466.460772] [] ret_from_intr+0x0/0x31 [ 466.460780] [] cpuidle_enter+0x17/0x20 [ 466.460790] [] call_cpuidle+0x32/0x60 [ 466.460800] [] cpu_startup_entry+0x2b8/0x3f0 [ 466.460807] [] rest_init+0x13a/0x140 [ 466.460817] [] start_kernel+0x4a3/0x4c4 [ 466.460827] [] x86_64_start_reservations+0x2a/0x2c [ 466.460837] [] x86_64_start_kernel+0x14a/0x16d [ 466.460846] irq event stamp: 754913 [ 466.460851] hardirqs last enabled at (754913): [] _raw_spin_unlock_irq+0x2c/0x40 [ 466.460861] hardirqs last disabled at (754912): [] _raw_spin_lock_irq+0x1d/0x60 [ 466.460869] softirqs last enabled at (753024): [] __do_softirq+0x380/0x490 [ 466.460880] softirqs last disabled at (753009): [] irq_exit+0x10f/0x120 [ 466.460888] other info that might help us debug this: [ 466.460894] Possible unsafe locking scenario: [ 466.460899] CPU0 [ 466.460903] ---- [ 466.460907] lock(&(&data->rxlock)->rlock); [ 466.460915] [ 466.460918] lock(&(&data->rxlock)->rlock); [ 466.460926] *** DEADLOCK *** [ 466.460935] 2 locks held by kworker/0:2/94: [ 466.460939] #0: ("events"){.+.+.+}, at: [] process_one_work+0x16b/0x660 [ 466.460958] #1: ((&data->work)){+.+...}, at: [] process_one_work+0x16b/0x660 [ 466.460974] Signed-off-by: Kuba Pawlak Signed-off-by: Marcel Holtmann --- drivers/bluetooth/btusb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index e33dacf..92f0ee3 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -1372,6 +1372,8 @@ static void btusb_work(struct work_struct *work) } if (data->isoc_altsetting != new_alts) { + unsigned long flags; + clear_bit(BTUSB_ISOC_RUNNING, &data->flags); usb_kill_anchored_urbs(&data->isoc_anchor); @@ -1384,10 +1386,10 @@ static void btusb_work(struct work_struct *work) * Clear outstanding fragment when selecting a new * alternate setting. */ - spin_lock(&data->rxlock); + spin_lock_irqsave(&data->rxlock, flags); kfree_skb(data->sco_skb); data->sco_skb = NULL; - spin_unlock(&data->rxlock); + spin_unlock_irqrestore(&data->rxlock, flags); if (__set_isoc_interface(hdev, new_alts) < 0) return; -- cgit v1.1 From 2ab216a7a9ca89d77388ad3f22a31f752dec5897 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Sun, 1 Nov 2015 09:39:48 +0100 Subject: Bluetooth: Check for supported white list before issuing commands The white list commands might not be implemented if the controller does not actually support the white list. So check the supported commands first before issuing these commands. Not supporting the white list is the same as supporting a white list with zero size. Signed-off-by: Marcel Holtmann Signed-off-by: Johan Hedberg --- net/bluetooth/hci_core.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 83a6aac..62edbf1 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -508,12 +508,6 @@ static void le_setup(struct hci_request *req) /* Read LE Supported States */ hci_req_add(req, HCI_OP_LE_READ_SUPPORTED_STATES, 0, NULL); - /* Read LE White List Size */ - hci_req_add(req, HCI_OP_LE_READ_WHITE_LIST_SIZE, 0, NULL); - - /* Clear LE White List */ - hci_req_add(req, HCI_OP_LE_CLEAR_WHITE_LIST, 0, NULL); - /* LE-only controllers have LE implicitly enabled */ if (!lmp_bredr_capable(hdev)) hci_dev_set_flag(hdev, HCI_LE_ENABLED); @@ -832,6 +826,17 @@ static void hci_init3_req(struct hci_request *req, unsigned long opt) hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL); } + if (hdev->commands[26] & 0x40) { + /* Read LE White List Size */ + hci_req_add(req, HCI_OP_LE_READ_WHITE_LIST_SIZE, + 0, NULL); + } + + if (hdev->commands[26] & 0x80) { + /* Clear LE White List */ + hci_req_add(req, HCI_OP_LE_CLEAR_WHITE_LIST, 0, NULL); + } + if (hdev->le_features[0] & HCI_LE_DATA_LEN_EXT) { /* Read LE Maximum Data Length */ hci_req_add(req, HCI_OP_LE_READ_MAX_DATA_LEN, 0, NULL); -- cgit v1.1 From 8a7889cc6e2dbbace114130f4efd9b77452069cd Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 2 Nov 2015 14:39:15 +0200 Subject: Bluetooth: L2CAP: Fix returning correct LE CoC response codes The core spec defines specific response codes for situations when the received CID is incorrect. Add the defines for these and return them as appropriate from the LE Connect Request handler function. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- include/net/bluetooth/l2cap.h | 2 ++ net/bluetooth/l2cap_core.c | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index c98afc0..5289929 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -275,6 +275,8 @@ struct l2cap_conn_rsp { #define L2CAP_CR_AUTHORIZATION 0x0006 #define L2CAP_CR_BAD_KEY_SIZE 0x0007 #define L2CAP_CR_ENCRYPTION 0x0008 +#define L2CAP_CR_INVALID_SCID 0x0009 +#define L2CAP_CR_SCID_IN_USE 0x0010 /* connect/create channel status */ #define L2CAP_CS_NO_INFO 0x0000 diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 7c65ee2..8fd36f5 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -5437,9 +5437,16 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, goto response_unlock; } + /* Check for valid dynamic CID range */ + if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_LE_DYN_END) { + result = L2CAP_CR_INVALID_SCID; + chan = NULL; + goto response_unlock; + } + /* Check if we already have channel with that dcid */ if (__l2cap_get_chan_by_dcid(conn, scid)) { - result = L2CAP_CR_NO_MEM; + result = L2CAP_CR_SCID_IN_USE; chan = NULL; goto response_unlock; } -- cgit v1.1 From ab0c127fbb21c19adb34b78ba26b84748d0cd4de Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 2 Nov 2015 14:39:16 +0200 Subject: Bluetooth: L2CAP: Fix checked range when allocating new CID The 'dyn_end' value is also a valid CID so it should be included in the range of values checked. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 8fd36f5..fdb7989 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -239,7 +239,7 @@ static u16 l2cap_alloc_cid(struct l2cap_conn *conn) else dyn_end = L2CAP_CID_DYN_END; - for (cid = L2CAP_CID_DYN_START; cid < dyn_end; cid++) { + for (cid = L2CAP_CID_DYN_START; cid <= dyn_end; cid++) { if (!__l2cap_get_chan_by_scid(conn, cid)) return cid; } -- cgit v1.1 From 40624183c202278e7e0edd01d1273efc87ddd1f2 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 2 Nov 2015 14:39:17 +0200 Subject: Bluetooth: L2CAP: Add missing checks for invalid LE DCID When receiving a connect response we should make sure that the DCID is within the valid range and that we don't already have another channel allocated for the same DCID. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index fdb7989..66e8b6e 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -5250,7 +5250,9 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn, credits = __le16_to_cpu(rsp->credits); result = __le16_to_cpu(rsp->result); - if (result == L2CAP_CR_SUCCESS && (mtu < 23 || mps < 23)) + if (result == L2CAP_CR_SUCCESS && (mtu < 23 || mps < 23 || + dcid < L2CAP_CID_DYN_START || + dcid > L2CAP_CID_LE_DYN_END)) return -EPROTO; BT_DBG("dcid 0x%4.4x mtu %u mps %u credits %u result 0x%2.2x", @@ -5270,6 +5272,11 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn, switch (result) { case L2CAP_CR_SUCCESS: + if (__l2cap_get_chan_by_dcid(conn, dcid)) { + err = -EBADSLT; + break; + } + chan->ident = 0; chan->dcid = dcid; chan->omtu = mtu; -- cgit v1.1