From 039fada5cd1963c32ed13d18d0dd467fdf966b66 Mon Sep 17 00:00:00 2001 From: Chan-yeol Park Date: Fri, 31 Oct 2014 14:23:06 +0900 Subject: Bluetooth: Fix hci_sync missing wakeup interrupt __hci_cmd_sync_ev(), __hci_req_sync() could miss wake_up_interrupt from hci_req_sync_complete() because hci_cmd_work() workqueue and its response could be completed before they are ready to get the signal through add_wait_queue(), set_current_state(TASK_INTERRUPTIBLE). Signed-off-by: Chan-yeol Park Signed-off-by: Kyungmin Park Signed-off-by: Johan Hedberg --- net/bluetooth/hci_core.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 91995f8ab0a0..41b147c36d11 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1147,13 +1147,15 @@ struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen, hdev->req_status = HCI_REQ_PEND; - err = hci_req_run(&req, hci_req_sync_complete); - if (err < 0) - return ERR_PTR(err); - add_wait_queue(&hdev->req_wait_q, &wait); set_current_state(TASK_INTERRUPTIBLE); + err = hci_req_run(&req, hci_req_sync_complete); + if (err < 0) { + remove_wait_queue(&hdev->req_wait_q, &wait); + return ERR_PTR(err); + } + schedule_timeout(timeout); remove_wait_queue(&hdev->req_wait_q, &wait); @@ -1211,10 +1213,15 @@ static int __hci_req_sync(struct hci_dev *hdev, func(&req, opt); + add_wait_queue(&hdev->req_wait_q, &wait); + set_current_state(TASK_INTERRUPTIBLE); + err = hci_req_run(&req, hci_req_sync_complete); if (err < 0) { hdev->req_status = 0; + remove_wait_queue(&hdev->req_wait_q, &wait); + /* ENODATA means the HCI request command queue is empty. * This can happen when a request with conditionals doesn't * trigger any commands to be sent. This is normal behavior @@ -1226,9 +1233,6 @@ static int __hci_req_sync(struct hci_dev *hdev, return err; } - add_wait_queue(&hdev->req_wait_q, &wait); - set_current_state(TASK_INTERRUPTIBLE); - schedule_timeout(timeout); remove_wait_queue(&hdev->req_wait_q, &wait); -- cgit v1.2.3 From 8761f9d6620b0be6b08f21807568fbbfcbb128d2 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Sun, 2 Nov 2014 02:45:58 +0100 Subject: Bluetooth: Check status of command complete for HCI_Reset When the HCI_Reset command returns, the status needs to be checked. It is unlikely that HCI_Reset actually fails, but when it fails, it is a bad idea to reset all values since the controller will have not reset its values in that case. Signed-off-by: Marcel Holtmann Signed-off-by: Johan Hedberg --- net/bluetooth/hci_event.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'net/bluetooth') diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index aa152140c3e2..3dd2550b4c07 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -189,6 +189,9 @@ static void hci_cc_reset(struct hci_dev *hdev, struct sk_buff *skb) clear_bit(HCI_RESET, &hdev->flags); + if (status) + return; + /* Reset all non-persistent flags */ hdev->dev_flags &= ~HCI_PERSISTENT_MASK; -- cgit v1.2.3 From 24dfa343716a493472db0555342bb88678efa444 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Sun, 2 Nov 2014 02:56:41 +0100 Subject: Bluetooth: Print error message for HCI_Hardware_Error event When the HCI_Hardware_Error event is send by the controller or injected by the driver, then at least print an error message. Signed-off-by: Marcel Holtmann Signed-off-by: Johan Hedberg --- include/net/bluetooth/hci.h | 5 +++++ net/bluetooth/hci_event.c | 11 +++++++++++ 2 files changed, 16 insertions(+) (limited to 'net/bluetooth') diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 6e8f24967308..ecfa306e1375 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -1463,6 +1463,11 @@ struct hci_ev_cmd_status { __le16 opcode; } __packed; +#define HCI_EV_HARDWARE_ERROR 0x10 +struct hci_ev_hardware_error { + __u8 code; +} __packed; + #define HCI_EV_ROLE_CHANGE 0x12 struct hci_ev_role_change { __u8 status; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 3dd2550b4c07..2f02ff0ed781 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2925,6 +2925,13 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb) } } +static void hci_hardware_error_evt(struct hci_dev *hdev, struct sk_buff *skb) +{ + struct hci_ev_hardware_error *ev = (void *) skb->data; + + BT_ERR("%s hardware error 0x%2.2x", hdev->name, ev->code); +} + static void hci_role_change_evt(struct hci_dev *hdev, struct sk_buff *skb) { struct hci_ev_role_change *ev = (void *) skb->data; @@ -4746,6 +4753,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb) hci_cmd_status_evt(hdev, skb); break; + case HCI_EV_HARDWARE_ERROR: + hci_hardware_error_evt(hdev, skb); + break; + case HCI_EV_ROLE_CHANGE: hci_role_change_evt(hdev, skb); break; -- cgit v1.2.3 From 65efd2bf4885312b42de9829159789199221cc60 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Sun, 2 Nov 2014 03:32:25 +0100 Subject: Bluetooth: Introduce BT_BREDR and BT_LE config options The current kernel options do not make it clear which modules are for Bluetooth Classic (BR/EDR) and which are for Bluetooth Low Energy (LE). To make it really clear, introduce BT_BREDR and BT_LE options with proper dependencies into the different modules. Both new options default to y to not create a regression with previous kernel config files. Signed-off-by: Marcel Holtmann Signed-off-by: Johan Hedberg --- net/bluetooth/Kconfig | 20 +++++++++++++++----- net/bluetooth/bnep/Kconfig | 2 +- net/bluetooth/cmtp/Kconfig | 2 +- net/bluetooth/hidp/Kconfig | 2 +- net/bluetooth/rfcomm/Kconfig | 2 +- 5 files changed, 19 insertions(+), 9 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig index 600fb29288f4..5e97a8ff850b 100644 --- a/net/bluetooth/Kconfig +++ b/net/bluetooth/Kconfig @@ -39,11 +39,10 @@ menuconfig BT to Bluetooth kernel modules are provided in the BlueZ packages. For more information, see . -config BT_6LOWPAN - tristate "Bluetooth 6LoWPAN support" - depends on BT && 6LOWPAN - help - IPv6 compression over Bluetooth Low Energy. +config BT_BREDR + bool "Bluetooth Classic (BR/EDR) features" + depends on BT + default y source "net/bluetooth/rfcomm/Kconfig" @@ -53,4 +52,15 @@ source "net/bluetooth/cmtp/Kconfig" source "net/bluetooth/hidp/Kconfig" +config BT_LE + bool "Bluetooth Low Energy (LE) features" + depends on BT + default y + +config BT_6LOWPAN + tristate "Bluetooth 6LoWPAN support" + depends on BT_LE && 6LOWPAN + help + IPv6 compression over Bluetooth Low Energy. + source "drivers/bluetooth/Kconfig" diff --git a/net/bluetooth/bnep/Kconfig b/net/bluetooth/bnep/Kconfig index 71791fc9f6b1..9b70317c49dc 100644 --- a/net/bluetooth/bnep/Kconfig +++ b/net/bluetooth/bnep/Kconfig @@ -1,6 +1,6 @@ config BT_BNEP tristate "BNEP protocol support" - depends on BT + depends on BT_BREDR select CRC32 help BNEP (Bluetooth Network Encapsulation Protocol) is Ethernet diff --git a/net/bluetooth/cmtp/Kconfig b/net/bluetooth/cmtp/Kconfig index 94cbf42ce155..939da0fbdd88 100644 --- a/net/bluetooth/cmtp/Kconfig +++ b/net/bluetooth/cmtp/Kconfig @@ -1,6 +1,6 @@ config BT_CMTP tristate "CMTP protocol support" - depends on BT && ISDN_CAPI + depends on BT_BREDR && ISDN_CAPI help CMTP (CAPI Message Transport Protocol) is a transport layer for CAPI messages. CMTP is required for the Bluetooth Common diff --git a/net/bluetooth/hidp/Kconfig b/net/bluetooth/hidp/Kconfig index 9332bc7aa851..bc8610b24077 100644 --- a/net/bluetooth/hidp/Kconfig +++ b/net/bluetooth/hidp/Kconfig @@ -1,6 +1,6 @@ config BT_HIDP tristate "HIDP protocol support" - depends on BT && INPUT + depends on BT_BREDR && INPUT select HID help HIDP (Human Interface Device Protocol) is a transport layer diff --git a/net/bluetooth/rfcomm/Kconfig b/net/bluetooth/rfcomm/Kconfig index 18d352ea2bc7..335df7515220 100644 --- a/net/bluetooth/rfcomm/Kconfig +++ b/net/bluetooth/rfcomm/Kconfig @@ -1,6 +1,6 @@ config BT_RFCOMM tristate "RFCOMM protocol support" - depends on BT + depends on BT_BREDR help RFCOMM provides connection oriented stream transport. RFCOMM support is required for Dialup Networking, OBEX and other Bluetooth -- cgit v1.2.3 From 75e0569f7fc22272ec5e3b99bf94c6f0ad43b35f Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Sun, 2 Nov 2014 08:15:38 +0100 Subject: Bluetooth: Add hci_reset_dev() for driver triggerd stack reset Some Bluetooth drivers require to reset the upper stack. To avoid having all drivers send HCI Hardware Error events, provide a generic function to wrap the reset functionality. Signed-off-by: Marcel Holtmann Signed-off-by: Johan Hedberg --- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_core.c | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) (limited to 'net/bluetooth') diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index b8685a77a15e..27ddb905b351 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -856,6 +856,7 @@ int hci_register_dev(struct hci_dev *hdev); void hci_unregister_dev(struct hci_dev *hdev); int hci_suspend_dev(struct hci_dev *hdev); int hci_resume_dev(struct hci_dev *hdev); +int hci_reset_dev(struct hci_dev *hdev); int hci_dev_open(__u16 dev); int hci_dev_close(__u16 dev); int hci_dev_reset(__u16 dev); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 41b147c36d11..a12e018ee21c 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -4248,6 +4248,24 @@ int hci_resume_dev(struct hci_dev *hdev) } EXPORT_SYMBOL(hci_resume_dev); +/* Reset HCI device */ +int hci_reset_dev(struct hci_dev *hdev) +{ + const u8 hw_err[] = { HCI_EV_HARDWARE_ERROR, 0x01, 0x00 }; + struct sk_buff *skb; + + skb = bt_skb_alloc(3, GFP_ATOMIC); + if (!skb) + return -ENOMEM; + + bt_cb(skb)->pkt_type = HCI_EVENT_PKT; + memcpy(skb_put(skb, 3), hw_err, 3); + + /* Send Hardware Error to upper stack */ + return hci_recv_frame(hdev, skb); +} +EXPORT_SYMBOL(hci_reset_dev); + /* Receive frame from HCI drivers */ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb) { -- cgit v1.2.3 From 40f4938aa6bfe2d792c0665c16d9dd15a5c1b119 Mon Sep 17 00:00:00 2001 From: Marcel Holtmann Date: Sun, 2 Nov 2014 21:46:52 +0100 Subject: Bluetooth: Consolidate whitelist debugfs entry into device_list The debufs entry for the BR/EDR whitelist is confusing since there is a controller debugfs entry with the name white_list and both are two different things. With the BR/EDR whitelist, the actual interface in use is the device list and thus just include all values from the internal BR/EDR whitelist in the device_list debugfs entry. Signed-off-by: Marcel Holtmann Signed-off-by: Johan Hedberg --- net/bluetooth/hci_core.c | 36 ++++++------------------------------ 1 file changed, 6 insertions(+), 30 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index a12e018ee21c..6c162c8809cf 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -200,31 +200,6 @@ static const struct file_operations blacklist_fops = { .release = single_release, }; -static int whitelist_show(struct seq_file *f, void *p) -{ - struct hci_dev *hdev = f->private; - struct bdaddr_list *b; - - hci_dev_lock(hdev); - list_for_each_entry(b, &hdev->whitelist, list) - seq_printf(f, "%pMR (type %u)\n", &b->bdaddr, b->bdaddr_type); - hci_dev_unlock(hdev); - - return 0; -} - -static int whitelist_open(struct inode *inode, struct file *file) -{ - return single_open(file, whitelist_show, inode->i_private); -} - -static const struct file_operations whitelist_fops = { - .open = whitelist_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - static int uuids_show(struct seq_file *f, void *p) { struct hci_dev *hdev = f->private; @@ -1030,10 +1005,13 @@ static int device_list_show(struct seq_file *f, void *ptr) { struct hci_dev *hdev = f->private; struct hci_conn_params *p; + struct bdaddr_list *b; hci_dev_lock(hdev); + list_for_each_entry(b, &hdev->whitelist, list) + seq_printf(f, "%pMR (type %u)\n", &b->bdaddr, b->bdaddr_type); list_for_each_entry(p, &hdev->le_conn_params, list) { - seq_printf(f, "%pMR %u %u\n", &p->addr, p->addr_type, + seq_printf(f, "%pMR (type %u) %u\n", &p->addr, p->addr_type, p->auto_connect); } hci_dev_unlock(hdev); @@ -1815,10 +1793,10 @@ static int __hci_init(struct hci_dev *hdev) &hdev->manufacturer); debugfs_create_u8("hci_version", 0444, hdev->debugfs, &hdev->hci_ver); debugfs_create_u16("hci_revision", 0444, hdev->debugfs, &hdev->hci_rev); + debugfs_create_file("device_list", 0444, hdev->debugfs, hdev, + &device_list_fops); debugfs_create_file("blacklist", 0444, hdev->debugfs, hdev, &blacklist_fops); - debugfs_create_file("whitelist", 0444, hdev->debugfs, hdev, - &whitelist_fops); debugfs_create_file("uuids", 0444, hdev->debugfs, hdev, &uuids_fops); debugfs_create_file("conn_info_min_age", 0644, hdev->debugfs, hdev, @@ -1897,8 +1875,6 @@ static int __hci_init(struct hci_dev *hdev) hdev, &adv_min_interval_fops); debugfs_create_file("adv_max_interval", 0644, hdev->debugfs, hdev, &adv_max_interval_fops); - debugfs_create_file("device_list", 0444, hdev->debugfs, hdev, - &device_list_fops); debugfs_create_u16("discov_interleaved_timeout", 0644, hdev->debugfs, &hdev->discov_interleaved_timeout); -- cgit v1.2.3 From a736abc1ac09b824387fb75b2aa7887c6e3ed68a Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Mon, 3 Nov 2014 14:20:56 +0100 Subject: Bluetooth: Fix invalid response for 'Start Discovery' command According to Management Interface API 'Start Discovery' command should generate a Command Complete event on failure. Currently kernel is sending Command Status on early errors. This results in userspace ignoring such event due to invalid size. bluetoothd[28499]: src/adapter.c:trigger_start_discovery() bluetoothd[28499]: src/adapter.c:cancel_passive_scanning() bluetoothd[28499]: src/adapter.c:start_discovery_timeout() bluetoothd[28499]: src/adapter.c:start_discovery_complete() status 0x0a bluetoothd[28499]: Wrong size of start discovery return parameters Reported-by: Jukka Taimisto Signed-off-by: Szymon Janc Signed-off-by: Johan Hedberg --- net/bluetooth/mgmt.c | 56 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 21 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 9c4daf715cf8..ce0272c6f71f 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -3727,20 +3727,23 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev, hci_dev_lock(hdev); if (!hdev_is_powered(hdev)) { - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY, - MGMT_STATUS_NOT_POWERED); + err = cmd_complete(sk, hdev->id, MGMT_OP_START_DISCOVERY, + MGMT_STATUS_NOT_POWERED, + &cp->type, sizeof(cp->type)); goto failed; } if (test_bit(HCI_PERIODIC_INQ, &hdev->dev_flags)) { - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY, - MGMT_STATUS_BUSY); + err = cmd_complete(sk, hdev->id, MGMT_OP_START_DISCOVERY, + MGMT_STATUS_BUSY, &cp->type, + sizeof(cp->type)); goto failed; } if (hdev->discovery.state != DISCOVERY_STOPPED) { - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY, - MGMT_STATUS_BUSY); + err = cmd_complete(sk, hdev->id, MGMT_OP_START_DISCOVERY, + MGMT_STATUS_BUSY, &cp->type, + sizeof(cp->type)); goto failed; } @@ -3758,15 +3761,18 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev, case DISCOV_TYPE_BREDR: status = mgmt_bredr_support(hdev); if (status) { - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY, - status); + err = cmd_complete(sk, hdev->id, + MGMT_OP_START_DISCOVERY, status, + &cp->type, sizeof(cp->type)); mgmt_pending_remove(cmd); goto failed; } if (test_bit(HCI_INQUIRY, &hdev->flags)) { - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY, - MGMT_STATUS_BUSY); + err = cmd_complete(sk, hdev->id, + MGMT_OP_START_DISCOVERY, + MGMT_STATUS_BUSY, &cp->type, + sizeof(cp->type)); mgmt_pending_remove(cmd); goto failed; } @@ -3783,16 +3789,19 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev, case DISCOV_TYPE_INTERLEAVED: status = mgmt_le_support(hdev); if (status) { - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY, - status); + err = cmd_complete(sk, hdev->id, + MGMT_OP_START_DISCOVERY, status, + &cp->type, sizeof(cp->type)); mgmt_pending_remove(cmd); goto failed; } if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED && !test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) { - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY, - MGMT_STATUS_NOT_SUPPORTED); + err = cmd_complete(sk, hdev->id, + MGMT_OP_START_DISCOVERY, + MGMT_STATUS_NOT_SUPPORTED, + &cp->type, sizeof(cp->type)); mgmt_pending_remove(cmd); goto failed; } @@ -3804,9 +3813,11 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev, */ if (hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT)) { - err = cmd_status(sk, hdev->id, - MGMT_OP_START_DISCOVERY, - MGMT_STATUS_REJECTED); + err = cmd_complete(sk, hdev->id, + MGMT_OP_START_DISCOVERY, + MGMT_STATUS_REJECTED, + &cp->type, + sizeof(cp->type)); mgmt_pending_remove(cmd); goto failed; } @@ -3829,8 +3840,10 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev, */ err = hci_update_random_address(&req, true, &own_addr_type); if (err < 0) { - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY, - MGMT_STATUS_FAILED); + err = cmd_complete(sk, hdev->id, + MGMT_OP_START_DISCOVERY, + MGMT_STATUS_FAILED, + &cp->type, sizeof(cp->type)); mgmt_pending_remove(cmd); goto failed; } @@ -3850,8 +3863,9 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev, break; default: - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY, - MGMT_STATUS_INVALID_PARAMS); + err = cmd_complete(sk, hdev->id, MGMT_OP_START_DISCOVERY, + MGMT_STATUS_INVALID_PARAMS, + &cp->type, sizeof(cp->type)); mgmt_pending_remove(cmd); goto failed; } -- cgit v1.2.3 From 2a68c897246b70a0c6b51a4a7d48d19b56b3e76b Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 4 Nov 2014 08:45:45 +0200 Subject: Bluetooth: Fix sparse warnings in RFCOMM This patch fixes the following sparse warnings in rfcomm/core.c: net/bluetooth/rfcomm/core.c:391:16: warning: dubious: x | !y net/bluetooth/rfcomm/core.c:546:24: warning: dubious: x | !y Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/rfcomm/core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c index bce9c3d39324..64e20dde4837 100644 --- a/net/bluetooth/rfcomm/core.c +++ b/net/bluetooth/rfcomm/core.c @@ -81,6 +81,8 @@ static struct rfcomm_session *rfcomm_session_del(struct rfcomm_session *s); #define __test_cr(b) (!!(b & 0x02)) #define __test_pf(b) (!!(b & 0x10)) +#define __session_dir(s) ((s)->initiator ? 0x00 : 0x01) + #define __addr(cr, dlci) (((dlci & 0x3f) << 2) | (cr << 1) | 0x01) #define __ctrl(type, pf) (((type & 0xef) | (pf << 4))) #define __dlci(dir, chn) (((chn & 0x1f) << 1) | dir) @@ -388,7 +390,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst, return err; } - dlci = __dlci(!s->initiator, channel); + dlci = __dlci(__session_dir(s), channel); /* Check if DLCI already exists */ if (rfcomm_dlc_get(s, dlci)) @@ -543,7 +545,7 @@ struct rfcomm_dlc *rfcomm_dlc_exists(bdaddr_t *src, bdaddr_t *dst, u8 channel) rfcomm_lock(); s = rfcomm_session_get(src, dst); if (s) { - dlci = __dlci(!s->initiator, channel); + dlci = __dlci(__session_dir(s), channel); dlc = rfcomm_dlc_get(s, dlci); } rfcomm_unlock(); -- cgit v1.2.3 From 50fc85f1b04908301c463e5b963f16d18a90d5cc Mon Sep 17 00:00:00 2001 From: Kuba Pawlak Date: Thu, 6 Nov 2014 19:36:52 +0100 Subject: Bluetooth: Clear role switch pending flag If role switch was rejected by the controller and HCI Event: Command Status returned with status "Command Disallowed" (0x0C) the flag HCI_CONN_RSWITCH_PEND remains set. No further role switches are possible as this flag prevents us from sending any new HCI Switch Role requests and the only way to clear it is to receive a valid HCI Event Switch Role. This patch clears the flag if command was rejected. 2013-01-01 00:03:44.209913 < HCI Command: Switch Role (0x02|0x000b) plen 7 bdaddr BC:C6:DB:C4:6F:79 role 0x00 Role: Master 2013-01-01 00:03:44.210867 > HCI Event: Command Status (0x0f) plen 4 Switch Role (0x02|0x000b) status 0x0c ncmd 1 Error: Command Disallowed Signed-off-by: Kuba Pawlak Signed-off-by: Marcel Holtmann --- net/bluetooth/hci_event.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'net/bluetooth') diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 2f02ff0ed781..73d9bb4a2c1e 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1947,6 +1947,29 @@ unlock: hci_dev_unlock(hdev); } +static void hci_cs_switch_role(struct hci_dev *hdev, u8 status) +{ + struct hci_cp_switch_role *cp; + struct hci_conn *conn; + + BT_DBG("%s status 0x%2.2x", hdev->name, status); + + if (!status) + return; + + cp = hci_sent_cmd_data(hdev, HCI_OP_SWITCH_ROLE); + if (!cp) + return; + + hci_dev_lock(hdev); + + conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->bdaddr); + if (conn) + clear_bit(HCI_CONN_RSWITCH_PEND, &conn->flags); + + hci_dev_unlock(hdev); +} + static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) { __u8 status = *((__u8 *) skb->data); @@ -2886,6 +2909,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb) hci_cs_exit_sniff_mode(hdev, ev->status); break; + case HCI_OP_SWITCH_ROLE: + hci_cs_switch_role(hdev, ev->status); + break; + case HCI_OP_DISCONNECT: hci_cs_disconnect(hdev, ev->status); break; -- cgit v1.2.3 From 9645c76c7c233da82ff7aced0177c8a131a51e70 Mon Sep 17 00:00:00 2001 From: Kuba Pawlak Date: Thu, 6 Nov 2014 19:36:53 +0100 Subject: Bluetooth: Sort switch cases by opcode's numeric value Opcodes in switch/case in hci_cmd_status_evt are not sorted by value. This patch restores proper ordering. Signed-off-by: Kuba Pawlak Signed-off-by: Marcel Holtmann --- net/bluetooth/hci_event.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 73d9bb4a2c1e..5e7be804c709 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2873,6 +2873,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb) hci_cs_create_conn(hdev, ev->status); break; + case HCI_OP_DISCONNECT: + hci_cs_disconnect(hdev, ev->status); + break; + case HCI_OP_ADD_SCO: hci_cs_add_sco(hdev, ev->status); break; @@ -2901,6 +2905,14 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb) hci_cs_setup_sync_conn(hdev, ev->status); break; + case HCI_OP_CREATE_PHY_LINK: + hci_cs_create_phylink(hdev, ev->status); + break; + + case HCI_OP_ACCEPT_PHY_LINK: + hci_cs_accept_phylink(hdev, ev->status); + break; + case HCI_OP_SNIFF_MODE: hci_cs_sniff_mode(hdev, ev->status); break; @@ -2913,18 +2925,6 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb) hci_cs_switch_role(hdev, ev->status); break; - case HCI_OP_DISCONNECT: - hci_cs_disconnect(hdev, ev->status); - break; - - case HCI_OP_CREATE_PHY_LINK: - hci_cs_create_phylink(hdev, ev->status); - break; - - case HCI_OP_ACCEPT_PHY_LINK: - hci_cs_accept_phylink(hdev, ev->status); - break; - case HCI_OP_LE_CREATE_CONN: hci_cs_le_create_conn(hdev, ev->status); break; -- cgit v1.2.3 From 56b2c3eea398c772dd895dc62c18cbdd1ba127b1 Mon Sep 17 00:00:00 2001 From: Martin Townsend Date: Thu, 6 Nov 2014 19:15:13 +0000 Subject: 6lowpan: move skb_free from error paths in decompression Currently we ensure that the skb is freed on every error path in IPHC decompression which makes it easy to introduce skb leaks. By centralising the skb_free into the receive function it makes future decompression routines easier to maintain. It does come at the expense of ensuring that the skb passed into the decompression routine must not be copied. Signed-off-by: Martin Townsend Acked-by: Jukka Rissanen Acked-by: Alexander Aring Signed-off-by: Marcel Holtmann --- net/6lowpan/iphc.c | 31 ++++++++++++------------------- net/bluetooth/6lowpan.c | 15 +++++++-------- net/ieee802154/6lowpan_rtnl.c | 16 ++++++---------- 3 files changed, 25 insertions(+), 37 deletions(-) (limited to 'net/bluetooth') diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c index cd5f8b8e34cd..aced97db62f0 100644 --- a/net/6lowpan/iphc.c +++ b/net/6lowpan/iphc.c @@ -319,7 +319,7 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, if (iphc1 & LOWPAN_IPHC_CID) { pr_debug("CID flag is set, increase header with one\n"); if (lowpan_fetch_skb(skb, &num_context, sizeof(num_context))) - goto drop; + return -EINVAL; } hdr.version = 6; @@ -331,7 +331,7 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, */ case 0: /* 00b */ if (lowpan_fetch_skb(skb, &tmp, sizeof(tmp))) - goto drop; + return -EINVAL; memcpy(&hdr.flow_lbl, &skb->data[0], 3); skb_pull(skb, 3); @@ -344,7 +344,7 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, */ case 2: /* 10b */ if (lowpan_fetch_skb(skb, &tmp, sizeof(tmp))) - goto drop; + return -EINVAL; hdr.priority = ((tmp >> 2) & 0x0f); hdr.flow_lbl[0] = ((tmp << 6) & 0xC0) | ((tmp >> 2) & 0x30); @@ -354,7 +354,7 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, */ case 1: /* 01b */ if (lowpan_fetch_skb(skb, &tmp, sizeof(tmp))) - goto drop; + return -EINVAL; hdr.flow_lbl[0] = (skb->data[0] & 0x0F) | ((tmp >> 2) & 0x30); memcpy(&hdr.flow_lbl[1], &skb->data[0], 2); @@ -371,7 +371,7 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, if ((iphc0 & LOWPAN_IPHC_NH_C) == 0) { /* Next header is carried inline */ if (lowpan_fetch_skb(skb, &hdr.nexthdr, sizeof(hdr.nexthdr))) - goto drop; + return -EINVAL; pr_debug("NH flag is set, next header carried inline: %02x\n", hdr.nexthdr); @@ -383,7 +383,7 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, } else { if (lowpan_fetch_skb(skb, &hdr.hop_limit, sizeof(hdr.hop_limit))) - goto drop; + return -EINVAL; } /* Extract SAM to the tmp variable */ @@ -402,7 +402,7 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, /* Check on error of previous branch */ if (err) - goto drop; + return -EINVAL; /* Extract DAM to the tmp variable */ tmp = ((iphc1 & LOWPAN_IPHC_DAM_11) >> LOWPAN_IPHC_DAM_BIT) & 0x03; @@ -417,7 +417,7 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, tmp); if (err) - goto drop; + return -EINVAL; } } else { err = uncompress_addr(skb, &hdr.daddr, tmp, daddr, @@ -425,7 +425,7 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, pr_debug("dest: stateless compression mode %d dest %pI6c\n", tmp, &hdr.daddr); if (err) - goto drop; + return -EINVAL; } /* UDP data uncompression */ @@ -434,16 +434,14 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, const int needed = sizeof(struct udphdr) + sizeof(hdr); if (uncompress_udp_header(skb, &uh)) - goto drop; + return -EINVAL; /* replace the compressed UDP head by the uncompressed UDP * header */ err = skb_cow(skb, needed); - if (unlikely(err)) { - kfree_skb(skb); + if (unlikely(err)) return err; - } skb_push(skb, sizeof(struct udphdr)); skb_reset_transport_header(skb); @@ -455,10 +453,8 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, hdr.nexthdr = UIP_PROTO_UDP; } else { err = skb_cow(skb, sizeof(hdr)); - if (unlikely(err)) { - kfree_skb(skb); + if (unlikely(err)) return err; - } } hdr.payload_len = htons(skb->len); @@ -478,9 +474,6 @@ lowpan_header_decompress(struct sk_buff *skb, struct net_device *dev, raw_dump_table(__func__, "raw header dump", (u8 *)&hdr, sizeof(hdr)); return 0; -drop: - kfree_skb(skb); - return -EINVAL; } EXPORT_SYMBOL_GPL(lowpan_header_decompress); diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index eef298d17452..dc23c55f1ab6 100644 --- a/net/bluetooth/6lowpan.c +++ b/net/bluetooth/6lowpan.c @@ -294,20 +294,20 @@ static int iphc_decompress(struct sk_buff *skb, struct net_device *netdev, peer = __peer_lookup_chan(dev, chan); rcu_read_unlock(); if (!peer) - goto drop; + return -EINVAL; saddr = peer->eui64_addr; daddr = dev->netdev->dev_addr; /* at least two bytes will be used for the encoding */ if (skb->len < 2) - goto drop; + return -EINVAL; if (lowpan_fetch_skb_u8(skb, &iphc0)) - goto drop; + return -EINVAL; if (lowpan_fetch_skb_u8(skb, &iphc1)) - goto drop; + return -EINVAL; return lowpan_header_decompress(skb, netdev, saddr, IEEE802154_ADDR_LONG, @@ -315,9 +315,6 @@ static int iphc_decompress(struct sk_buff *skb, struct net_device *netdev, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN, iphc0, iphc1); -drop: - kfree_skb(skb); - return -EINVAL; } static int recv_pkt(struct sk_buff *skb, struct net_device *dev, @@ -370,8 +367,10 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev, goto drop; ret = iphc_decompress(local_skb, dev, chan); - if (ret < 0) + if (ret < 0) { + kfree_skb(local_skb); goto drop; + } local_skb->protocol = htons(ETH_P_IPV6); local_skb->pkt_type = PACKET_HOST; diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c index a96b64c9a73d..290e14f2e92e 100644 --- a/net/ieee802154/6lowpan_rtnl.c +++ b/net/ieee802154/6lowpan_rtnl.c @@ -176,13 +176,13 @@ iphc_decompress(struct sk_buff *skb, const struct ieee802154_hdr *hdr) raw_dump_table(__func__, "raw skb data dump", skb->data, skb->len); /* at least two bytes will be used for the encoding */ if (skb->len < 2) - goto drop; + return -EINVAL; if (lowpan_fetch_skb_u8(skb, &iphc0)) - goto drop; + return -EINVAL; if (lowpan_fetch_skb_u8(skb, &iphc1)) - goto drop; + return -EINVAL; ieee802154_addr_to_sa(&sa, &hdr->source); ieee802154_addr_to_sa(&da, &hdr->dest); @@ -200,10 +200,6 @@ iphc_decompress(struct sk_buff *skb, const struct ieee802154_hdr *hdr) return lowpan_header_decompress(skb, skb->dev, sap, sa.addr_type, IEEE802154_ADDR_LEN, dap, da.addr_type, IEEE802154_ADDR_LEN, iphc0, iphc1); - -drop: - kfree_skb(skb); - return -EINVAL; } static struct sk_buff* @@ -522,7 +518,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev, case LOWPAN_DISPATCH_IPHC: /* ipv6 datagram */ ret = iphc_decompress(skb, &hdr); if (ret < 0) - goto drop; + goto drop_skb; return lowpan_give_skb_to_devices(skb, NULL); case LOWPAN_DISPATCH_FRAG1: /* first fragment header */ @@ -530,7 +526,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev, if (ret == 1) { ret = iphc_decompress(skb, &hdr); if (ret < 0) - goto drop; + goto drop_skb; return lowpan_give_skb_to_devices(skb, NULL); } else if (ret == -1) { @@ -543,7 +539,7 @@ static int lowpan_rcv(struct sk_buff *skb, struct net_device *dev, if (ret == 1) { ret = iphc_decompress(skb, &hdr); if (ret < 0) - goto drop; + goto drop_skb; return lowpan_give_skb_to_devices(skb, NULL); } else if (ret == -1) { -- cgit v1.2.3 From cb77c3ec075a50e9f956f62dc2e4c0394df1d578 Mon Sep 17 00:00:00 2001 From: Jaganath Kanakkassery Date: Fri, 7 Nov 2014 16:39:09 +0530 Subject: Bluetooth: Send mgmt_connected only if state is BT_CONFIG If a remote name request is initiated while acl connection is going on, and if it fails then mgmt_connected will be sent. Evetually after acl connection, authentication will not be initiated and userspace will never get pairing reply. < HCI Command: Create Connection (0x01|0x0005) plen 13 bdaddr AA:BB:CC:DD:EE:FF ptype 0xcc18 rswitch 0x01 clkoffset 0x2306 (valid) Packet type: DM1 DM3 DM5 DH1 DH3 DH5 > HCI Event: Command Status (0x0f) plen 4 Create Connection (0x01|0x0005) status 0x00 ncmd 1 > HCI Event: Inquiry Complete (0x01) plen 1 status 0x00 < HCI Command: Remote Name Request (0x01|0x0019) plen 10 bdaddr AA:BB:CC:DD:EE:FF mode 1 clkoffset 0x2306 > HCI Event: Command Status (0x0f) plen 4 Remote Name Request (0x01|0x0019) status 0x0c ncmd 1 Error: Command Disallowed > HCI Event: Connect Complete (0x03) plen 11 status 0x00 handle 50 bdaddr 00:0D:FD:47:53:B2 type ACL encrypt 0x00 < HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2 handle 50 > HCI Event: Command Status (0x0f) plen 4 Read Remote Supported Features (0x01|0x001b) status 0x00 ncmd 1 > HCI Event: Max Slots Change (0x1b) plen 3 handle 50 slots 5 > HCI Event: Read Remote Supported Features (0x0b) plen 11 status 0x00 handle 50 Features: 0xff 0xff 0x8f 0xfe 0x9b 0xff 0x59 0x83 < HCI Command: Read Remote Extended Features (0x01|0x001c) plen 3 handle 50 page 1 > HCI Event: Command Status (0x0f) plen 4 Read Remote Extended Features (0x01|0x001c) status 0x00 ncmd 1 > HCI Event: Read Remote Extended Features (0x23) plen 13 status 0x00 handle 50 page 1 max 1 Features: 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00 This patch sends mgmt_connected in remote name command status only if conn->state is BT_CONFIG Signed-off-by: Jaganath Kanakkassery Signed-off-by: Johan Hedberg --- net/bluetooth/hci_event.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 5e7be804c709..68c882fd20fd 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1581,7 +1581,8 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn, struct discovery_state *discov = &hdev->discovery; struct inquiry_entry *e; - if (conn && !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags)) + if (conn && conn->state == BT_CONFIG && + !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags)) mgmt_device_connected(hdev, conn, 0, name, name_len); if (discov->state == DISCOVERY_STOPPED) -- cgit v1.2.3 From b0c42cd7b210efc74aa4bfc3e39a2814dfaa9b89 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Wed, 8 Oct 2014 10:24:53 +0200 Subject: Bluetooth: 6lowpan: fix skb_unshare behaviour This patch reverts commit: a7807d73 ("Bluetooth: 6lowpan: Avoid memory leak if memory allocation fails") which was wrong suggested by Alexander Aring. The function skb_unshare run also kfree_skb on failure. Signed-off-by: Alexander Aring Signed-off-by: Marcel Holtmann Cc: stable@vger.kernel.org # 3.18.x --- net/bluetooth/6lowpan.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index dc23c55f1ab6..3f20dce9d671 100644 --- a/net/bluetooth/6lowpan.c +++ b/net/bluetooth/6lowpan.c @@ -614,17 +614,13 @@ static netdev_tx_t bt_xmit(struct sk_buff *skb, struct net_device *netdev) int err = 0; bdaddr_t addr; u8 addr_type; - struct sk_buff *tmpskb; /* We must take a copy of the skb before we modify/replace the ipv6 * header as the header could be used elsewhere */ - tmpskb = skb_unshare(skb, GFP_ATOMIC); - if (!tmpskb) { - kfree_skb(skb); + skb = skb_unshare(skb, GFP_ATOMIC); + if (!skb) return NET_XMIT_DROP; - } - skb = tmpskb; /* Return values from setup_header() * <0 - error, packet is dropped -- cgit v1.2.3 From 252670c421c785127cb55db03c48df5feb57ce12 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 10 Nov 2014 15:53:45 +0200 Subject: Bluetooth: Fix sparse warning in amp.c This fixes the following sparse warning: net/bluetooth/amp.c:152:53: warning: Variable length array is used. The warning itself is probably harmless since this kind of usage of shash_desc is present also in other places in the kernel (there's even a convenience macro SHASH_DESC_ON_STACK available for defining such stack variables). However, dynamically allocated versions are also used in several places of the kernel (e.g. kernel/kexec.c and lib/digsig.c) which have the benefit of not exhibiting the sparse warning. Since there are no more sparse warnings in the Bluetooth subsystem after fixing this one it is now easier to spot whenever new ones might get introduced by future patches. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/amp.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c index 2640d78f30b8..ee016f039100 100644 --- a/net/bluetooth/amp.c +++ b/net/bluetooth/amp.c @@ -134,6 +134,7 @@ struct hci_conn *phylink_add(struct hci_dev *hdev, struct amp_mgr *mgr, static int hmac_sha256(u8 *key, u8 ksize, char *plaintext, u8 psize, u8 *output) { struct crypto_shash *tfm; + struct shash_desc *shash; int ret; if (!ksize) @@ -148,18 +149,24 @@ static int hmac_sha256(u8 *key, u8 ksize, char *plaintext, u8 psize, u8 *output) ret = crypto_shash_setkey(tfm, key, ksize); if (ret) { BT_DBG("crypto_ahash_setkey failed: err %d", ret); - } else { - char desc[sizeof(struct shash_desc) + - crypto_shash_descsize(tfm)] CRYPTO_MINALIGN_ATTR; - struct shash_desc *shash = (struct shash_desc *)desc; - - shash->tfm = tfm; - shash->flags = CRYPTO_TFM_REQ_MAY_SLEEP; + goto failed; + } - ret = crypto_shash_digest(shash, plaintext, psize, - output); + shash = kzalloc(sizeof(*shash) + crypto_shash_descsize(tfm), + GFP_KERNEL); + if (!shash) { + ret = -ENOMEM; + goto failed; } + shash->tfm = tfm; + shash->flags = CRYPTO_TFM_REQ_MAY_SLEEP; + + ret = crypto_shash_digest(shash, plaintext, psize, output); + + kfree(shash); + +failed: crypto_free_shash(tfm); return ret; } -- cgit v1.2.3 From 60cb49d2c92969f7b0e7da863fc0cbe3ec0c715c Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 11 Nov 2014 11:33:24 +0200 Subject: Bluetooth: Fix mgmt connected notification This patch fixes a regression that was introduced by commit cb77c3ec075a50e9f956f62dc2e4c0394df1d578. In addition to BT_CONFIG, BT_CONNECTED is also a state in which we may get a remote name and need to indicate over mgmt the connection status. This scenario is particularly likely to happen for incoming connections that do not need authentication since there the hci_conn state will reach BT_CONNECTED before the remote name is received. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/hci_event.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 68c882fd20fd..aec3b1dce1cc 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1581,7 +1581,13 @@ static void hci_check_pending_name(struct hci_dev *hdev, struct hci_conn *conn, struct discovery_state *discov = &hdev->discovery; struct inquiry_entry *e; - if (conn && conn->state == BT_CONFIG && + /* Update the mgmt connected state if necessary. Be careful with + * conn objects that exist but are not (yet) connected however. + * Only those in BT_CONFIG or BT_CONNECTED states can be + * considered connected. + */ + if (conn && + (conn->state == BT_CONFIG || conn->state == BT_CONNECTED) && !test_and_set_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags)) mgmt_device_connected(hdev, conn, 0, name, name_len); -- cgit v1.2.3 From 4e7902267708e5a389398d9014455b2ed4892912 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 11 Nov 2014 14:16:29 +0200 Subject: Bluetooth: 6lowpan: Remove unnecessary RCU callback When kfree() is all that's needed to free an object protected by RCU there's a kfree_rcu() convenience function that can be used. This patch updates the 6lowpan code to use this, thereby eliminating the need for the separate peer_free() function. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/6lowpan.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index 3f20dce9d671..3d8ceb251d75 100644 --- a/net/bluetooth/6lowpan.c +++ b/net/bluetooth/6lowpan.c @@ -87,13 +87,6 @@ struct lowpan_dev { struct delayed_work notify_peers; }; -static inline void peer_free(struct rcu_head *head) -{ - struct lowpan_peer *e = container_of(head, struct lowpan_peer, rcu); - - kfree(e); -} - static inline struct lowpan_dev *lowpan_dev(const struct net_device *netdev) { return netdev_priv(netdev); @@ -108,7 +101,7 @@ static inline void peer_add(struct lowpan_dev *dev, struct lowpan_peer *peer) static inline bool peer_del(struct lowpan_dev *dev, struct lowpan_peer *peer) { list_del_rcu(&peer->list); - call_rcu(&peer->rcu, peer_free); + kfree_rcu(peer, rcu); module_put(THIS_MODULE); @@ -1219,7 +1212,7 @@ static void disconnect_all_peers(void) l2cap_chan_close(peer->chan, ENOENT); list_del_rcu(&peer->list); - call_rcu(&peer->rcu, peer_free); + kfree_rcu(peer, rcu); module_put(THIS_MODULE); } -- cgit v1.2.3 From f03567040cbf874834c9e3e52b72fdcb672b9bbb Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Wed, 12 Nov 2014 09:15:50 +0200 Subject: Bluetooth: Fix l2cap_sock_teardown_cb lockdep warning Any code calling bt_accept_dequeue() to get a new child socket from a server socket should use lock_sock_nested to avoid lockdep warnings due to the parent and child sockets being locked at the same time. The l2cap_sock_accept() function is already doing this correctly but a second place calling bt_accept_dequeue() is the code path from l2cap_sock_teardown_cb() that calls l2cap_sock_cleanup_listen(). This patch fixes the proper nested locking annotation and thereby avoids the following style of lockdep warning. [ +0.000224] [ INFO: possible recursive locking detected ] [ +0.000222] 3.17.0+ #1153 Not tainted [ +0.000130] --------------------------------------------- [ +0.000227] l2cap-tester/562 is trying to acquire lock: [ +0.000210] (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+...}, at: [] bt_accept_dequeue+0x68/0x11b [ +0.000467] but task is already holding lock: [ +0.000186] (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+...}, at: [] lock_sock+0xa/0xc [ +0.000421] other info that might help us debug this: [ +0.000199] Possible unsafe locking scenario: [ +0.000117] CPU0 [ +0.000000] ---- [ +0.000000] lock(sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP); [ +0.000000] lock(sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP); [ +0.000000] *** DEADLOCK *** Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_sock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 31f106e61ca2..ad1cf82fee02 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1246,7 +1246,7 @@ static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err) struct sock *sk = chan->data; struct sock *parent; - lock_sock(sk); + lock_sock_nested(sk, SINGLE_DEPTH_NESTING); parent = bt_sk(sk)->parent; -- cgit v1.2.3 From a930430b047a0cc118bfc47ca54fcdfbadf091d2 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Wed, 12 Nov 2014 09:17:08 +0200 Subject: Bluetooth: Remove unnecessary hci_dev_lock/unlock in smp.c The mgmt_user_passkey_request and related functions do not do anything else except read access to hdev->id. This member never changes after the hdev creation so there is no need to acquire a lock to read it. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/smp.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 3ebf65b50881..3d38553eb526 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -514,8 +514,6 @@ static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth, set_bit(SMP_FLAG_TK_VALID, &smp->flags); } - hci_dev_lock(hcon->hdev); - if (method == REQ_PASSKEY) ret = mgmt_user_passkey_request(hcon->hdev, &hcon->dst, hcon->type, hcon->dst_type); @@ -528,8 +526,6 @@ static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth, hcon->type, hcon->dst_type, passkey, 0); - hci_dev_unlock(hcon->hdev); - return ret; } -- cgit v1.2.3 From abe84903a8efc6b83fa92161429e0e3a28bde15c Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Wed, 12 Nov 2014 22:22:21 +0200 Subject: Bluetooth: Use proper nesting annotation for l2cap_chan lock By default lockdep considers all L2CAP channels equal. This would mean that we get warnings if a channel is locked when another one's lock is tried to be acquired in the same thread. This kind of inter-channel locking dependencies exist in the form of parent-child channels as well as any channel wishing to elevate the security by requesting procedures on the SMP channel. To eliminate the chance for these lockdep warnings we introduce a nesting level for each channel and use that when acquiring the channel lock. For now there exists the earlier mentioned three identified categories: SMP, "normal" channels and parent channels (i.e. those in BT_LISTEN state). The nesting level is defined as atomic_t since we need access to it before the lock is actually acquired. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- include/net/bluetooth/l2cap.h | 15 ++++++++++++++- net/bluetooth/l2cap_sock.c | 9 +++++++++ net/bluetooth/smp.c | 10 ++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) (limited to 'net/bluetooth') diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index ead99f032f7a..061e648052c8 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -28,6 +28,7 @@ #define __L2CAP_H #include +#include /* L2CAP defaults */ #define L2CAP_DEFAULT_MTU 672 @@ -481,6 +482,7 @@ struct l2cap_chan { struct hci_conn *hs_hcon; struct hci_chan *hs_hchan; struct kref kref; + atomic_t nesting; __u8 state; @@ -713,6 +715,17 @@ enum { FLAG_HOLD_HCI_CONN, }; +/* Lock nesting levels for L2CAP channels. We need these because lockdep + * otherwise considers all channels equal and will e.g. complain about a + * connection oriented channel triggering SMP procedures or a listening + * channel creating and locking a child channel. + */ +enum { + L2CAP_NESTING_SMP, + L2CAP_NESTING_NORMAL, + L2CAP_NESTING_PARENT, +}; + enum { L2CAP_TX_STATE_XMIT, L2CAP_TX_STATE_WAIT_F, @@ -778,7 +791,7 @@ void l2cap_chan_put(struct l2cap_chan *c); static inline void l2cap_chan_lock(struct l2cap_chan *chan) { - mutex_lock(&chan->lock); + mutex_lock_nested(&chan->lock, atomic_read(&chan->nesting)); } static inline void l2cap_chan_unlock(struct l2cap_chan *chan) diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index ad1cf82fee02..f1a51564b8fd 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -285,6 +285,12 @@ static int l2cap_sock_listen(struct socket *sock, int backlog) sk->sk_max_ack_backlog = backlog; sk->sk_ack_backlog = 0; + /* Listening channels need to use nested locking in order not to + * cause lockdep warnings when the created child channels end up + * being locked in the same thread as the parent channel. + */ + atomic_set(&chan->nesting, L2CAP_NESTING_PARENT); + chan->state = BT_LISTEN; sk->sk_state = BT_LISTEN; @@ -1497,6 +1503,9 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent) l2cap_chan_set_defaults(chan); } + /* Set default lock nesting level */ + atomic_set(&chan->nesting, L2CAP_NESTING_NORMAL); + /* Default config options */ chan->flush_to = L2CAP_DEFAULT_FLUSH_TO; diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 3d38553eb526..3b63c7f09dd5 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -1658,6 +1658,13 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) chan->omtu = pchan->omtu; chan->mode = pchan->mode; + /* Other L2CAP channels may request SMP routines in order to + * change the security level. This means that the SMP channel + * lock must be considered in its own category to avoid lockdep + * warnings. + */ + atomic_set(&chan->nesting, L2CAP_NESTING_SMP); + BT_DBG("created chan %p", chan); return chan; @@ -1715,6 +1722,9 @@ int smp_register(struct hci_dev *hdev) chan->imtu = L2CAP_DEFAULT_MTU; chan->ops = &smp_root_chan_ops; + /* Set correct nesting level for a parent/listening channel */ + atomic_set(&chan->nesting, L2CAP_NESTING_PARENT); + hdev->smp_data = chan; return 0; -- cgit v1.2.3 From 3b2ab39e26c90aac947f120b0e27c5277c660d79 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Wed, 12 Nov 2014 22:22:22 +0200 Subject: Bluetooth: Fix L2CAP socket lock nesting level The teardown callback for L2CAP channels is problematic in that it is explicitly called for all types of channels from l2cap_chan_del(), meaning it's not possible to hard-code a nesting level when taking the socket lock. The simplest way to have a correct nesting level for the socket locking is to use the same value as for the chan. This also means that the other places trying to lock parent sockets need to be update to use the chan value (since L2CAP_NESTING_PARENT is defined as 2 whereas SINGLE_DEPTH_NESTING has the value 1). Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_sock.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index f1a51564b8fd..7913c28c643d 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -307,7 +307,7 @@ static int l2cap_sock_accept(struct socket *sock, struct socket *newsock, long timeo; int err = 0; - lock_sock_nested(sk, SINGLE_DEPTH_NESTING); + lock_sock_nested(sk, L2CAP_NESTING_PARENT); timeo = sock_rcvtimeo(sk, flags & O_NONBLOCK); @@ -339,7 +339,7 @@ static int l2cap_sock_accept(struct socket *sock, struct socket *newsock, release_sock(sk); timeo = schedule_timeout(timeo); - lock_sock_nested(sk, SINGLE_DEPTH_NESTING); + lock_sock_nested(sk, L2CAP_NESTING_PARENT); } __set_current_state(TASK_RUNNING); remove_wait_queue(sk_sleep(sk), &wait); @@ -1252,7 +1252,14 @@ static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err) struct sock *sk = chan->data; struct sock *parent; - lock_sock_nested(sk, SINGLE_DEPTH_NESTING); + /* This callback can be called both for server (BT_LISTEN) + * sockets as well as "normal" ones. To avoid lockdep warnings + * with child socket locking (through l2cap_sock_cleanup_listen) + * we need separation into separate nesting levels. The simplest + * way to accomplish this is to inherit the nesting level used + * for the channel. + */ + lock_sock_nested(sk, atomic_read(&chan->nesting)); parent = bt_sk(sk)->parent; -- cgit v1.2.3 From ff714119a6d2e2fc3c2e046d77801afa83a9ace2 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 13 Nov 2014 09:46:04 +0200 Subject: Bluetooth: Fix L2CAP nesting level initialization location There's no reason why all users of L2CAP would need to worry about initializing chan->nesting to L2CAP_NESTING_NORMAL (which is important since 0 is the same as NESTING_SMP). This patch moves the initialization to the common place that's used to create all new channels, i.e. the l2cap_chan_create() function. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_core.c | 3 +++ net/bluetooth/l2cap_sock.c | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index fc15174c612c..52e1871d6334 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -424,6 +424,9 @@ struct l2cap_chan *l2cap_chan_create(void) mutex_init(&chan->lock); + /* Set default lock nesting level */ + atomic_set(&chan->nesting, L2CAP_NESTING_NORMAL); + write_lock(&chan_list_lock); list_add(&chan->global_l, &chan_list); write_unlock(&chan_list_lock); diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 7913c28c643d..a5aa9f92b5e2 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1510,9 +1510,6 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent) l2cap_chan_set_defaults(chan); } - /* Set default lock nesting level */ - atomic_set(&chan->nesting, L2CAP_NESTING_NORMAL); - /* Default config options */ chan->flush_to = L2CAP_DEFAULT_FLUSH_TO; -- cgit v1.2.3 From 2773b024229bab23ef36e198e0555630f74f23ef Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 13 Nov 2014 09:46:05 +0200 Subject: Bluetooth: Fix correct nesting for 6lowpan server channel Server channels in BT_LISTEN state should use L2CAP_NESTING_PARENT. This patch fixes the nesting value for the 6lowpan channel. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/6lowpan.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'net/bluetooth') diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index 3d8ceb251d75..bdcaefd2db12 100644 --- a/net/bluetooth/6lowpan.c +++ b/net/bluetooth/6lowpan.c @@ -1130,6 +1130,8 @@ static struct l2cap_chan *bt_6lowpan_listen(void) pchan->state = BT_LISTEN; pchan->src_type = BDADDR_LE_PUBLIC; + atomic_set(&pchan->nesting, L2CAP_NESTING_PARENT); + BT_DBG("psm 0x%04x chan %p src type %d", psm_6lowpan, pchan, pchan->src_type); -- cgit v1.2.3 From a809eff11f81e2dcf1e792586ef70693c60d7dc8 Mon Sep 17 00:00:00 2001 From: Fabian Frederick Date: Fri, 14 Nov 2014 19:35:05 +0100 Subject: Bluetooth: hidp: replace kzalloc/copy_from_user by memdup_user use memdup_user for rd_data import. Signed-off-by: Fabian Frederick Signed-off-by: Marcel Holtmann --- net/bluetooth/hidp/core.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 1b7d605706aa..cc25d0b74b36 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -736,14 +736,10 @@ static int hidp_setup_hid(struct hidp_session *session, struct hid_device *hid; int err; - session->rd_data = kzalloc(req->rd_size, GFP_KERNEL); - if (!session->rd_data) - return -ENOMEM; + session->rd_data = memdup_user(req->rd_data, req->rd_size); + if (IS_ERR(session->rd_data)) + return PTR_ERR(session->rd_data); - if (copy_from_user(session->rd_data, req->rd_data, req->rd_size)) { - err = -EFAULT; - goto fault; - } session->rd_size = req->rd_size; hid = hid_allocate_device(); -- cgit v1.2.3 From fa37c1aa30e538329b64dd55f401334f4bff47f5 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 13 Nov 2014 10:55:17 +0200 Subject: Bluetooth: Fix sending incorrect LE CoC PDU in BT_CONNECT2 state For LE CoC L2CAP servers we don't do security level elevation during the BT_CONNECT2 state (instead LE CoC simply sends an immediate error response if the security level isn't high enough). Therefore if we get a security level change while an LE CoC channel is in the BT_CONNECT2 state we should simply do nothing. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 52e1871d6334..76045497eaa1 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -7332,7 +7332,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) l2cap_start_connection(chan); else __set_chan_timer(chan, L2CAP_DISC_TIMEOUT); - } else if (chan->state == BT_CONNECT2) { + } else if (chan->state == BT_CONNECT2 && + chan->mode != L2CAP_MODE_LE_FLOWCTL) { struct l2cap_conn_rsp rsp; __u16 res, stat; -- cgit v1.2.3 From 35dc6f834c9dc888391c7b700130d0831a907ca1 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 13 Nov 2014 10:55:18 +0200 Subject: Bluetooth: Add key preference parameter to smp_sufficient_security So far smp_sufficient_security() has returned false if we're encrypted with an STK but do have an LTK available. However, for the sake of LE CoC servers we do want to let the incoming connection through even though we're only encrypted with the STK. This patch adds a key preference parameter to smp_sufficient_security() with two possible values (enum used instead of bool for readability). Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_core.c | 3 ++- net/bluetooth/smp.c | 20 +++++++++++--------- net/bluetooth/smp.h | 9 ++++++++- 3 files changed, 21 insertions(+), 11 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 76045497eaa1..a37f809591ad 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -5391,7 +5391,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, mutex_lock(&conn->chan_lock); l2cap_chan_lock(pchan); - if (!smp_sufficient_security(conn->hcon, pchan->sec_level)) { + if (!smp_sufficient_security(conn->hcon, pchan->sec_level, + SMP_ALLOW_STK)) { result = L2CAP_CR_AUTHENTICATION; chan = NULL; goto response_unlock; diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 3b63c7f09dd5..3dc5f0e66405 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -1122,18 +1122,20 @@ static bool smp_ltk_encrypt(struct l2cap_conn *conn, u8 sec_level) return true; } -bool smp_sufficient_security(struct hci_conn *hcon, u8 sec_level) +bool smp_sufficient_security(struct hci_conn *hcon, u8 sec_level, + enum smp_key_pref key_pref) { if (sec_level == BT_SECURITY_LOW) return true; - /* If we're encrypted with an STK always claim insufficient - * security. This way we allow the connection to be re-encrypted - * with an LTK, even if the LTK provides the same level of - * security. Only exception is if we don't have an LTK (e.g. - * because of key distribution bits). + /* If we're encrypted with an STK but the caller prefers using + * LTK claim insufficient security. This way we allow the + * connection to be re-encrypted with an LTK, even if the LTK + * provides the same level of security. Only exception is if we + * don't have an LTK (e.g. because of key distribution bits). */ - if (test_bit(HCI_CONN_STK_ENCRYPT, &hcon->flags) && + if (key_pref == SMP_USE_LTK && + test_bit(HCI_CONN_STK_ENCRYPT, &hcon->flags) && hci_find_ltk_by_addr(hcon->hdev, &hcon->dst, hcon->dst_type, hcon->role)) return false; @@ -1167,7 +1169,7 @@ static u8 smp_cmd_security_req(struct l2cap_conn *conn, struct sk_buff *skb) else sec_level = authreq_to_seclevel(auth); - if (smp_sufficient_security(hcon, sec_level)) + if (smp_sufficient_security(hcon, sec_level, SMP_USE_LTK)) return 0; if (sec_level > hcon->pending_sec_level) @@ -1217,7 +1219,7 @@ int smp_conn_security(struct hci_conn *hcon, __u8 sec_level) if (!test_bit(HCI_LE_ENABLED, &hcon->hdev->dev_flags)) return 1; - if (smp_sufficient_security(hcon, sec_level)) + if (smp_sufficient_security(hcon, sec_level, SMP_USE_LTK)) return 1; if (sec_level > hcon->pending_sec_level) diff --git a/net/bluetooth/smp.h b/net/bluetooth/smp.h index 86a683a8b491..f76083b85005 100644 --- a/net/bluetooth/smp.h +++ b/net/bluetooth/smp.h @@ -133,8 +133,15 @@ static inline u8 smp_ltk_sec_level(struct smp_ltk *key) return BT_SECURITY_MEDIUM; } +/* Key preferences for smp_sufficient security */ +enum smp_key_pref { + SMP_ALLOW_STK, + SMP_USE_LTK, +}; + /* SMP Commands */ -bool smp_sufficient_security(struct hci_conn *hcon, u8 sec_level); +bool smp_sufficient_security(struct hci_conn *hcon, u8 sec_level, + enum smp_key_pref key_pref); int smp_conn_security(struct hci_conn *hcon, __u8 sec_level); int smp_user_confirm_reply(struct hci_conn *conn, u16 mgmt_op, __le32 passkey); -- cgit v1.2.3 From 3e64b7bd8234b459134b3059919828122e2fd79f Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 13 Nov 2014 10:55:19 +0200 Subject: Bluetooth: Trigger SMP for the appropriate LE CoC errors The insufficient authentication/encryption errors indicate to the L2CAP client that it should try to elevate the security level. Since there really isn't any exception to this rule it makes sense to fully handle it on the kernel side instead of pushing the responsibility to user space. This patch adds special handling of these two error codes and calls smp_conn_security() with the elevated security level if necessary. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_core.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index a37f809591ad..15784d32108d 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -5218,9 +5218,10 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn, u8 *data) { struct l2cap_le_conn_rsp *rsp = (struct l2cap_le_conn_rsp *) data; + struct hci_conn *hcon = conn->hcon; u16 dcid, mtu, mps, credits, result; struct l2cap_chan *chan; - int err; + int err, sec_level; if (cmd_len < sizeof(*rsp)) return -EPROTO; @@ -5259,6 +5260,26 @@ static int l2cap_le_connect_rsp(struct l2cap_conn *conn, l2cap_chan_ready(chan); break; + case L2CAP_CR_AUTHENTICATION: + case L2CAP_CR_ENCRYPTION: + /* If we already have MITM protection we can't do + * anything. + */ + if (hcon->sec_level > BT_SECURITY_MEDIUM) { + l2cap_chan_del(chan, ECONNREFUSED); + break; + } + + sec_level = hcon->sec_level + 1; + if (chan->sec_level < sec_level) + chan->sec_level = sec_level; + + /* We'll need to send a new Connect Request */ + clear_bit(FLAG_LE_CONN_REQ_SENT, &chan->flags); + + smp_conn_security(hcon, chan->sec_level); + break; + default: l2cap_chan_del(chan, ECONNREFUSED); break; -- cgit v1.2.3 From 970d0f1b280372cfd46b6de5529d96f8448de943 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 13 Nov 2014 14:37:47 +0200 Subject: Bluetooth: Convert LTK list to RCU This patch set converts the hdev->long_term_keys list to use RCU to eliminate the need to use hci_dev_lock/unlock. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_core.c | 42 +++++++++++++++++++++++----------------- net/bluetooth/hci_event.c | 4 ++-- net/bluetooth/smp.c | 10 ++++------ 4 files changed, 31 insertions(+), 26 deletions(-) (limited to 'net/bluetooth') diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 4e39a5adfcab..a4adef22ad7c 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -108,6 +108,7 @@ struct smp_csrk { struct smp_ltk { struct list_head list; + struct rcu_head rcu; bdaddr_t bdaddr; u8 bdaddr_type; u8 authenticated; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 6c162c8809cf..c9495fb9f595 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -778,17 +778,15 @@ static const struct file_operations identity_resolving_keys_fops = { static int long_term_keys_show(struct seq_file *f, void *ptr) { struct hci_dev *hdev = f->private; - struct list_head *p, *n; + struct smp_ltk *ltk; - hci_dev_lock(hdev); - list_for_each_safe(p, n, &hdev->long_term_keys) { - struct smp_ltk *ltk = list_entry(p, struct smp_ltk, list); + rcu_read_lock(); + list_for_each_entry_rcu(ltk, &hdev->long_term_keys, list) seq_printf(f, "%pMR (type %u) %u 0x%02x %u %.4x %.16llx %*phN\n", <k->bdaddr, ltk->bdaddr_type, ltk->authenticated, ltk->type, ltk->enc_size, __le16_to_cpu(ltk->ediv), __le64_to_cpu(ltk->rand), 16, ltk->val); - } - hci_dev_unlock(hdev); + rcu_read_unlock(); return 0; } @@ -3106,11 +3104,11 @@ void hci_link_keys_clear(struct hci_dev *hdev) void hci_smp_ltks_clear(struct hci_dev *hdev) { - struct smp_ltk *k, *tmp; + struct smp_ltk *k; - list_for_each_entry_safe(k, tmp, &hdev->long_term_keys, list) { - list_del(&k->list); - kfree(k); + list_for_each_entry_rcu(k, &hdev->long_term_keys, list) { + list_del_rcu(&k->list); + kfree_rcu(k, rcu); } } @@ -3184,15 +3182,18 @@ struct smp_ltk *hci_find_ltk(struct hci_dev *hdev, __le16 ediv, __le64 rand, { struct smp_ltk *k; - list_for_each_entry(k, &hdev->long_term_keys, list) { + rcu_read_lock(); + list_for_each_entry_rcu(k, &hdev->long_term_keys, list) { if (k->ediv != ediv || k->rand != rand) continue; if (ltk_role(k->type) != role) continue; + rcu_read_unlock(); return k; } + rcu_read_unlock(); return NULL; } @@ -3202,11 +3203,16 @@ struct smp_ltk *hci_find_ltk_by_addr(struct hci_dev *hdev, bdaddr_t *bdaddr, { struct smp_ltk *k; - list_for_each_entry(k, &hdev->long_term_keys, list) + rcu_read_lock(); + list_for_each_entry_rcu(k, &hdev->long_term_keys, list) { if (addr_type == k->bdaddr_type && bacmp(bdaddr, &k->bdaddr) == 0 && - ltk_role(k->type) == role) + ltk_role(k->type) == role) { + rcu_read_unlock(); return k; + } + } + rcu_read_unlock(); return NULL; } @@ -3309,7 +3315,7 @@ struct smp_ltk *hci_add_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, key = kzalloc(sizeof(*key), GFP_KERNEL); if (!key) return NULL; - list_add(&key->list, &hdev->long_term_keys); + list_add_rcu(&key->list, &hdev->long_term_keys); } bacpy(&key->bdaddr, bdaddr); @@ -3365,17 +3371,17 @@ int hci_remove_link_key(struct hci_dev *hdev, bdaddr_t *bdaddr) int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 bdaddr_type) { - struct smp_ltk *k, *tmp; + struct smp_ltk *k; int removed = 0; - list_for_each_entry_safe(k, tmp, &hdev->long_term_keys, list) { + list_for_each_entry_rcu(k, &hdev->long_term_keys, list) { if (bacmp(bdaddr, &k->bdaddr) || k->bdaddr_type != bdaddr_type) continue; BT_DBG("%s removing %pMR", hdev->name, bdaddr); - list_del(&k->list); - kfree(k); + list_del_rcu(&k->list); + kfree_rcu(k, rcu); removed++; } diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index aec3b1dce1cc..09d76547d985 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -4578,8 +4578,8 @@ static void hci_le_ltk_request_evt(struct hci_dev *hdev, struct sk_buff *skb) */ if (ltk->type == SMP_STK) { set_bit(HCI_CONN_STK_ENCRYPT, &conn->flags); - list_del(<k->list); - kfree(ltk); + list_del_rcu(<k->list); + kfree_rcu(ltk, rcu); } else { clear_bit(HCI_CONN_STK_ENCRYPT, &conn->flags); } diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 3dc5f0e66405..fd2dfe5222bc 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -383,13 +383,13 @@ static void smp_chan_destroy(struct l2cap_conn *conn) /* If pairing failed clean up any keys we might have */ if (!complete) { if (smp->ltk) { - list_del(&smp->ltk->list); - kfree(smp->ltk); + list_del_rcu(&smp->ltk->list); + kfree_rcu(smp->ltk, rcu); } if (smp->slave_ltk) { - list_del(&smp->slave_ltk->list); - kfree(smp->slave_ltk); + list_del_rcu(&smp->slave_ltk->list); + kfree_rcu(smp->slave_ltk, rcu); } if (smp->remote_irk) { @@ -1321,7 +1321,6 @@ static int smp_cmd_master_ident(struct l2cap_conn *conn, struct sk_buff *skb) skb_pull(skb, sizeof(*rp)); - hci_dev_lock(hdev); authenticated = (hcon->sec_level == BT_SECURITY_HIGH); ltk = hci_add_ltk(hdev, &hcon->dst, hcon->dst_type, SMP_LTK, authenticated, smp->tk, smp->enc_key_size, @@ -1329,7 +1328,6 @@ static int smp_cmd_master_ident(struct l2cap_conn *conn, struct sk_buff *skb) smp->ltk = ltk; if (!(smp->remote_key_dist & KEY_DIST_MASK)) smp_distribute_keys(smp); - hci_dev_unlock(hdev); return 0; } -- cgit v1.2.3 From adae20cb2d20e5151b866945f802b0c2312f0f82 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 13 Nov 2014 14:37:48 +0200 Subject: Bluetooth: Convert IRK list to RCU This patch set converts the hdev->identity_resolving_keys list to use RCU to eliminate the need to use hci_dev_lock/unlock. An additional change that must be done is to remove use of CRYPTO_ALG_ASYNC for the hdev-specific AES crypto context. The reason is that this context is used for matching RPAs and the loop that does the matching is under the RCU read lock, i.e. is an atomic section which cannot sleep. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_core.c | 46 +++++++++++++++++++++++----------------- net/bluetooth/smp.c | 10 ++++----- 3 files changed, 33 insertions(+), 24 deletions(-) (limited to 'net/bluetooth') diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index a4adef22ad7c..fe2d5f299e12 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -121,6 +121,7 @@ struct smp_ltk { struct smp_irk { struct list_head list; + struct rcu_head rcu; bdaddr_t rpa; bdaddr_t bdaddr; u8 addr_type; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index c9495fb9f595..90ea0b7670d2 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -748,16 +748,15 @@ static const struct file_operations white_list_fops = { static int identity_resolving_keys_show(struct seq_file *f, void *ptr) { struct hci_dev *hdev = f->private; - struct list_head *p, *n; + struct smp_irk *irk; - hci_dev_lock(hdev); - list_for_each_safe(p, n, &hdev->identity_resolving_keys) { - struct smp_irk *irk = list_entry(p, struct smp_irk, list); + rcu_read_lock(); + list_for_each_entry_rcu(irk, &hdev->identity_resolving_keys, list) { seq_printf(f, "%pMR (type %u) %*phN %pMR\n", &irk->bdaddr, irk->addr_type, 16, irk->val, &irk->rpa); } - hci_dev_unlock(hdev); + rcu_read_unlock(); return 0; } @@ -3114,11 +3113,11 @@ void hci_smp_ltks_clear(struct hci_dev *hdev) void hci_smp_irks_clear(struct hci_dev *hdev) { - struct smp_irk *k, *tmp; + struct smp_irk *k; - list_for_each_entry_safe(k, tmp, &hdev->identity_resolving_keys, list) { - list_del(&k->list); - kfree(k); + list_for_each_entry_rcu(k, &hdev->identity_resolving_keys, list) { + list_del_rcu(&k->list); + kfree_rcu(k, rcu); } } @@ -3221,17 +3220,22 @@ struct smp_irk *hci_find_irk_by_rpa(struct hci_dev *hdev, bdaddr_t *rpa) { struct smp_irk *irk; - list_for_each_entry(irk, &hdev->identity_resolving_keys, list) { - if (!bacmp(&irk->rpa, rpa)) + rcu_read_lock(); + list_for_each_entry_rcu(irk, &hdev->identity_resolving_keys, list) { + if (!bacmp(&irk->rpa, rpa)) { + rcu_read_unlock(); return irk; + } } - list_for_each_entry(irk, &hdev->identity_resolving_keys, list) { + list_for_each_entry_rcu(irk, &hdev->identity_resolving_keys, list) { if (smp_irk_matches(hdev, irk->val, rpa)) { bacpy(&irk->rpa, rpa); + rcu_read_unlock(); return irk; } } + rcu_read_unlock(); return NULL; } @@ -3245,11 +3249,15 @@ struct smp_irk *hci_find_irk_by_addr(struct hci_dev *hdev, bdaddr_t *bdaddr, if (addr_type == ADDR_LE_DEV_RANDOM && (bdaddr->b[5] & 0xc0) != 0xc0) return NULL; - list_for_each_entry(irk, &hdev->identity_resolving_keys, list) { + rcu_read_lock(); + list_for_each_entry_rcu(irk, &hdev->identity_resolving_keys, list) { if (addr_type == irk->addr_type && - bacmp(bdaddr, &irk->bdaddr) == 0) + bacmp(bdaddr, &irk->bdaddr) == 0) { + rcu_read_unlock(); return irk; + } } + rcu_read_unlock(); return NULL; } @@ -3344,7 +3352,7 @@ struct smp_irk *hci_add_irk(struct hci_dev *hdev, bdaddr_t *bdaddr, bacpy(&irk->bdaddr, bdaddr); irk->addr_type = addr_type; - list_add(&irk->list, &hdev->identity_resolving_keys); + list_add_rcu(&irk->list, &hdev->identity_resolving_keys); } memcpy(irk->val, val, 16); @@ -3390,16 +3398,16 @@ int hci_remove_ltk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 bdaddr_type) void hci_remove_irk(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 addr_type) { - struct smp_irk *k, *tmp; + struct smp_irk *k; - list_for_each_entry_safe(k, tmp, &hdev->identity_resolving_keys, list) { + list_for_each_entry_rcu(k, &hdev->identity_resolving_keys, list) { if (bacmp(bdaddr, &k->bdaddr) || k->addr_type != addr_type) continue; BT_DBG("%s removing %pMR", hdev->name, bdaddr); - list_del(&k->list); - kfree(k); + list_del_rcu(&k->list); + kfree_rcu(k, rcu); } } diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index fd2dfe5222bc..7b610f615257 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -393,8 +393,8 @@ static void smp_chan_destroy(struct l2cap_conn *conn) } if (smp->remote_irk) { - list_del(&smp->remote_irk->list); - kfree(smp->remote_irk); + list_del_rcu(&smp->remote_irk->list); + kfree_rcu(smp->remote_irk, rcu); } } @@ -655,8 +655,8 @@ static void smp_notify_keys(struct l2cap_conn *conn) * just remove it. */ if (!bacmp(&smp->remote_irk->rpa, BDADDR_ANY)) { - list_del(&smp->remote_irk->list); - kfree(smp->remote_irk); + list_del_rcu(&smp->remote_irk->list); + kfree_rcu(smp->remote_irk, rcu); smp->remote_irk = NULL; } } @@ -1696,7 +1696,7 @@ int smp_register(struct hci_dev *hdev) BT_DBG("%s", hdev->name); - tfm_aes = crypto_alloc_blkcipher("ecb(aes)", 0, CRYPTO_ALG_ASYNC); + tfm_aes = crypto_alloc_blkcipher("ecb(aes)", 0, 0); if (IS_ERR(tfm_aes)) { int err = PTR_ERR(tfm_aes); BT_ERR("Unable to create crypto context"); -- cgit v1.2.3 From d88b5bbf1a985c338967f3c41351b32b747a55fe Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 13 Nov 2014 14:37:49 +0200 Subject: Bluetooth: Remove unnecessary hdev locking in smp.c Now that the SMP related key lists are converted to RCU there is nothing in smp_cmd_sign_info() or smp_cmd_ident_addr_info() that would require taking the hdev lock (including the smp_distribute_keys call). This patch removes this unnecessary locking. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/smp.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 7b610f615257..069b76e03b57 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -1374,8 +1374,6 @@ static int smp_cmd_ident_addr_info(struct l2cap_conn *conn, skb_pull(skb, sizeof(*info)); - hci_dev_lock(hcon->hdev); - /* Strictly speaking the Core Specification (4.1) allows sending * an empty address which would force us to rely on just the IRK * as "identity information". However, since such @@ -1403,8 +1401,6 @@ distribute: if (!(smp->remote_key_dist & KEY_DIST_MASK)) smp_distribute_keys(smp); - hci_dev_unlock(hcon->hdev); - return 0; } @@ -1413,7 +1409,6 @@ static int smp_cmd_sign_info(struct l2cap_conn *conn, struct sk_buff *skb) struct smp_cmd_sign_info *rp = (void *) skb->data; struct l2cap_chan *chan = conn->smp; struct smp_chan *smp = chan->data; - struct hci_dev *hdev = conn->hcon->hdev; struct smp_csrk *csrk; BT_DBG("conn %p", conn); @@ -1426,7 +1421,6 @@ static int smp_cmd_sign_info(struct l2cap_conn *conn, struct sk_buff *skb) skb_pull(skb, sizeof(*rp)); - hci_dev_lock(hdev); csrk = kzalloc(sizeof(*csrk), GFP_KERNEL); if (csrk) { csrk->master = 0x01; @@ -1434,7 +1428,6 @@ static int smp_cmd_sign_info(struct l2cap_conn *conn, struct sk_buff *skb) } smp->csrk = csrk; smp_distribute_keys(smp); - hci_dev_unlock(hdev); return 0; } -- cgit v1.2.3 From 49d1174130df596fcfec3b6a56dce8aa5b997f2d Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Thu, 13 Nov 2014 14:37:50 +0200 Subject: Bluetooth: Add debug logs to help track locking issues This patch adds some extra debug logs to L2CAP related code. These are mainly to help track locking issues but will probably be useful for debugging other types of issues as well. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap_core.c | 3 ++- net/bluetooth/l2cap_sock.c | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) (limited to 'net/bluetooth') diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 15784d32108d..8e1273173020 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -570,7 +570,8 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err) __clear_chan_timer(chan); - BT_DBG("chan %p, conn %p, err %d", chan, conn, err); + BT_DBG("chan %p, conn %p, err %d, state %s", chan, conn, err, + state_to_string(chan->state)); chan->ops->teardown(chan, err); diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index a5aa9f92b5e2..b0efb7202957 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1102,6 +1102,8 @@ static int l2cap_sock_shutdown(struct socket *sock, int how) chan = l2cap_pi(sk)->chan; conn = chan->conn; + BT_DBG("chan %p state %s", chan, state_to_string(chan->state)); + if (conn) mutex_lock(&conn->chan_lock); @@ -1159,12 +1161,16 @@ static void l2cap_sock_cleanup_listen(struct sock *parent) { struct sock *sk; - BT_DBG("parent %p", parent); + BT_DBG("parent %p state %s", parent, + state_to_string(parent->sk_state)); /* Close not yet accepted channels */ while ((sk = bt_accept_dequeue(parent, NULL))) { struct l2cap_chan *chan = l2cap_pi(sk)->chan; + BT_DBG("child chan %p state %s", chan, + state_to_string(chan->state)); + l2cap_chan_lock(chan); __clear_chan_timer(chan); l2cap_chan_close(chan, ECONNRESET); @@ -1252,6 +1258,8 @@ static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err) struct sock *sk = chan->data; struct sock *parent; + BT_DBG("chan %p state %s", chan, state_to_string(chan->state)); + /* This callback can be called both for server (BT_LISTEN) * sockets as well as "normal" ones. To avoid lockdep warnings * with child socket locking (through l2cap_sock_cleanup_listen) -- cgit v1.2.3 From eedbd5812c2afe79646a7c1c071875e46c867935 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Sat, 15 Nov 2014 09:34:23 +0200 Subject: Bluetooth: Fix clearing remote OOB data through mgmt When passed BDADDR_ANY the Remove Remote OOB Data comand is specified to clear all entries. This patch adds the necessary check and calls hci_remote_oob_data_clear() when necessary. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/mgmt.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'net/bluetooth') diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index ce0272c6f71f..b84c0923ec62 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -3635,12 +3635,19 @@ static int remove_remote_oob_data(struct sock *sk, struct hci_dev *hdev, hci_dev_lock(hdev); + if (!bacmp(&cp->addr.bdaddr, BDADDR_ANY)) { + hci_remote_oob_data_clear(hdev); + status = MGMT_STATUS_SUCCESS; + goto done; + } + err = hci_remove_remote_oob_data(hdev, &cp->addr.bdaddr); if (err < 0) status = MGMT_STATUS_INVALID_PARAMS; else status = MGMT_STATUS_SUCCESS; +done: err = cmd_complete(sk, hdev->id, MGMT_OP_REMOVE_REMOTE_OOB_DATA, status, &cp->addr, sizeof(cp->addr)); -- cgit v1.2.3 From c19a495c8bccc15acd299f26d72b5d51eb3acb1d Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 17 Nov 2014 20:52:19 +0200 Subject: Bluetooth: Fix BR/EDR-only address checks for remote OOB data For now the mgmt commands dealing with remote OOB data are strictly BR/EDR-only. This patch fixes missing checks for the passed address type so that any non-BR/EDR value triggers the appropriate error response. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/mgmt.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'net/bluetooth') diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index b84c0923ec62..258c9826e78c 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -3589,6 +3589,14 @@ static int add_remote_oob_data(struct sock *sk, struct hci_dev *hdev, struct mgmt_cp_add_remote_oob_data *cp = data; u8 status; + if (cp->addr.type != BDADDR_BREDR) { + err = cmd_complete(sk, hdev->id, + MGMT_OP_ADD_REMOTE_OOB_DATA, + MGMT_STATUS_INVALID_PARAMS, + &cp->addr, sizeof(cp->addr)); + goto unlock; + } + err = hci_add_remote_oob_data(hdev, &cp->addr.bdaddr, cp->hash, cp->randomizer); if (err < 0) @@ -3602,6 +3610,14 @@ static int add_remote_oob_data(struct sock *sk, struct hci_dev *hdev, struct mgmt_cp_add_remote_oob_ext_data *cp = data; u8 status; + if (cp->addr.type != BDADDR_BREDR) { + err = cmd_complete(sk, hdev->id, + MGMT_OP_ADD_REMOTE_OOB_DATA, + MGMT_STATUS_INVALID_PARAMS, + &cp->addr, sizeof(cp->addr)); + goto unlock; + } + err = hci_add_remote_oob_ext_data(hdev, &cp->addr.bdaddr, cp->hash192, cp->randomizer192, @@ -3620,6 +3636,7 @@ static int add_remote_oob_data(struct sock *sk, struct hci_dev *hdev, MGMT_STATUS_INVALID_PARAMS); } +unlock: hci_dev_unlock(hdev); return err; } @@ -3633,6 +3650,11 @@ static int remove_remote_oob_data(struct sock *sk, struct hci_dev *hdev, BT_DBG("%s", hdev->name); + if (cp->addr.type != BDADDR_BREDR) + return cmd_complete(sk, hdev->id, MGMT_OP_REMOVE_REMOTE_OOB_DATA, + MGMT_STATUS_INVALID_PARAMS, + &cp->addr, sizeof(cp->addr)); + hci_dev_lock(hdev); if (!bacmp(&cp->addr.bdaddr, BDADDR_ANY)) { -- cgit v1.2.3 From 38da1703060a520e69b00405f9bdf765d1396cd0 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 17 Nov 2014 20:52:20 +0200 Subject: Bluetooth: Use shorter "rand" name for "randomizer" The common short form of "randomizer" is "rand" in many places (including the Bluetooth specification). The shorter version also makes for easier to read code with less forced line breaks. This patch renames all occurences of "randomizer" to "rand" in the Bluetooth subsystem code. Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- include/net/bluetooth/hci.h | 12 ++++++------ include/net/bluetooth/hci_core.h | 14 +++++++------- include/net/bluetooth/mgmt.h | 12 ++++++------ net/bluetooth/hci_core.c | 14 +++++++------- net/bluetooth/hci_event.c | 17 +++++++---------- net/bluetooth/mgmt.c | 23 +++++++++-------------- 6 files changed, 42 insertions(+), 50 deletions(-) (limited to 'net/bluetooth') diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index d5f85d7746bc..e56f9099f8e3 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -639,7 +639,7 @@ struct hci_cp_user_passkey_reply { struct hci_cp_remote_oob_data_reply { bdaddr_t bdaddr; __u8 hash[16]; - __u8 randomizer[16]; + __u8 rand[16]; } __packed; #define HCI_OP_REMOTE_OOB_DATA_NEG_REPLY 0x0433 @@ -731,9 +731,9 @@ struct hci_rp_set_csb { struct hci_cp_remote_oob_ext_data_reply { bdaddr_t bdaddr; __u8 hash192[16]; - __u8 randomizer192[16]; + __u8 rand192[16]; __u8 hash256[16]; - __u8 randomizer256[16]; + __u8 rand256[16]; } __packed; #define HCI_OP_SNIFF_MODE 0x0803 @@ -940,7 +940,7 @@ struct hci_cp_write_ssp_mode { struct hci_rp_read_local_oob_data { __u8 status; __u8 hash[16]; - __u8 randomizer[16]; + __u8 rand[16]; } __packed; #define HCI_OP_READ_INQ_RSP_TX_POWER 0x0c58 @@ -1024,9 +1024,9 @@ struct hci_cp_write_sc_support { struct hci_rp_read_local_oob_ext_data { __u8 status; __u8 hash192[16]; - __u8 randomizer192[16]; + __u8 rand192[16]; __u8 hash256[16]; - __u8 randomizer256[16]; + __u8 rand256[16]; } __packed; #define HCI_OP_READ_LOCAL_VERSION 0x1001 diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index fe2d5f299e12..a805b3d97c0b 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -140,9 +140,9 @@ struct oob_data { struct list_head list; bdaddr_t bdaddr; u8 hash192[16]; - u8 randomizer192[16]; + u8 rand192[16]; u8 hash256[16]; - u8 randomizer256[16]; + u8 rand256[16]; }; #define HCI_MAX_SHORT_NAME_LENGTH 10 @@ -943,10 +943,10 @@ void hci_remote_oob_data_clear(struct hci_dev *hdev); struct oob_data *hci_find_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr); int hci_add_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr, - u8 *hash, u8 *randomizer); + u8 *hash, u8 *rand); int hci_add_remote_oob_ext_data(struct hci_dev *hdev, bdaddr_t *bdaddr, - u8 *hash192, u8 *randomizer192, - u8 *hash256, u8 *randomizer256); + u8 *hash192, u8 *rand192, + u8 *hash256, u8 *rand256); int hci_remove_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr); void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb); @@ -1374,8 +1374,8 @@ void mgmt_set_class_of_dev_complete(struct hci_dev *hdev, u8 *dev_class, u8 status); void mgmt_set_local_name_complete(struct hci_dev *hdev, u8 *name, u8 status); void mgmt_read_local_oob_data_complete(struct hci_dev *hdev, u8 *hash192, - u8 *randomizer192, u8 *hash256, - u8 *randomizer256, u8 status); + u8 *rand192, u8 *hash256, u8 *rand256, + u8 status); void mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type, u8 addr_type, u8 *dev_class, s8 rssi, u32 flags, u8 *eir, u16 eir_len, u8 *scan_rsp, u8 scan_rsp_len); diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h index 414cd2f9a437..b391fd663468 100644 --- a/include/net/bluetooth/mgmt.h +++ b/include/net/bluetooth/mgmt.h @@ -299,28 +299,28 @@ struct mgmt_cp_user_passkey_neg_reply { #define MGMT_READ_LOCAL_OOB_DATA_SIZE 0 struct mgmt_rp_read_local_oob_data { __u8 hash[16]; - __u8 randomizer[16]; + __u8 rand[16]; } __packed; struct mgmt_rp_read_local_oob_ext_data { __u8 hash192[16]; - __u8 randomizer192[16]; + __u8 rand192[16]; __u8 hash256[16]; - __u8 randomizer256[16]; + __u8 rand256[16]; } __packed; #define MGMT_OP_ADD_REMOTE_OOB_DATA 0x0021 struct mgmt_cp_add_remote_oob_data { struct mgmt_addr_info addr; __u8 hash[16]; - __u8 randomizer[16]; + __u8 rand[16]; } __packed; #define MGMT_ADD_REMOTE_OOB_DATA_SIZE (MGMT_ADDR_INFO_SIZE + 32) struct mgmt_cp_add_remote_oob_ext_data { struct mgmt_addr_info addr; __u8 hash192[16]; - __u8 randomizer192[16]; + __u8 rand192[16]; __u8 hash256[16]; - __u8 randomizer256[16]; + __u8 rand256[16]; } __packed; #define MGMT_ADD_REMOTE_OOB_EXT_DATA_SIZE (MGMT_ADDR_INFO_SIZE + 64) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 90ea0b7670d2..bbefb4eea36e 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3469,7 +3469,7 @@ void hci_remote_oob_data_clear(struct hci_dev *hdev) } int hci_add_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr, - u8 *hash, u8 *randomizer) + u8 *hash, u8 *rand) { struct oob_data *data; @@ -3484,10 +3484,10 @@ int hci_add_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr, } memcpy(data->hash192, hash, sizeof(data->hash192)); - memcpy(data->randomizer192, randomizer, sizeof(data->randomizer192)); + memcpy(data->rand192, rand, sizeof(data->rand192)); memset(data->hash256, 0, sizeof(data->hash256)); - memset(data->randomizer256, 0, sizeof(data->randomizer256)); + memset(data->rand256, 0, sizeof(data->rand256)); BT_DBG("%s for %pMR", hdev->name, bdaddr); @@ -3495,8 +3495,8 @@ int hci_add_remote_oob_data(struct hci_dev *hdev, bdaddr_t *bdaddr, } int hci_add_remote_oob_ext_data(struct hci_dev *hdev, bdaddr_t *bdaddr, - u8 *hash192, u8 *randomizer192, - u8 *hash256, u8 *randomizer256) + u8 *hash192, u8 *rand192, + u8 *hash256, u8 *rand256) { struct oob_data *data; @@ -3511,10 +3511,10 @@ int hci_add_remote_oob_ext_data(struct hci_dev *hdev, bdaddr_t *bdaddr, } memcpy(data->hash192, hash192, sizeof(data->hash192)); - memcpy(data->randomizer192, randomizer192, sizeof(data->randomizer192)); + memcpy(data->rand192, rand192, sizeof(data->rand192)); memcpy(data->hash256, hash256, sizeof(data->hash256)); - memcpy(data->randomizer256, randomizer256, sizeof(data->randomizer256)); + memcpy(data->rand256, rand256, sizeof(data->rand256)); BT_DBG("%s for %pMR", hdev->name, bdaddr); diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 09d76547d985..844f7d1ff1cd 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -994,8 +994,8 @@ static void hci_cc_read_local_oob_data(struct hci_dev *hdev, BT_DBG("%s status 0x%2.2x", hdev->name, rp->status); hci_dev_lock(hdev); - mgmt_read_local_oob_data_complete(hdev, rp->hash, rp->randomizer, - NULL, NULL, rp->status); + mgmt_read_local_oob_data_complete(hdev, rp->hash, rp->rand, NULL, NULL, + rp->status); hci_dev_unlock(hdev); } @@ -1007,8 +1007,8 @@ static void hci_cc_read_local_oob_ext_data(struct hci_dev *hdev, BT_DBG("%s status 0x%2.2x", hdev->name, rp->status); hci_dev_lock(hdev); - mgmt_read_local_oob_data_complete(hdev, rp->hash192, rp->randomizer192, - rp->hash256, rp->randomizer256, + mgmt_read_local_oob_data_complete(hdev, rp->hash192, rp->rand192, + rp->hash256, rp->rand256, rp->status); hci_dev_unlock(hdev); } @@ -3996,11 +3996,9 @@ static void hci_remote_oob_data_request_evt(struct hci_dev *hdev, bacpy(&cp.bdaddr, &ev->bdaddr); memcpy(cp.hash192, data->hash192, sizeof(cp.hash192)); - memcpy(cp.randomizer192, data->randomizer192, - sizeof(cp.randomizer192)); + memcpy(cp.rand192, data->rand192, sizeof(cp.rand192)); memcpy(cp.hash256, data->hash256, sizeof(cp.hash256)); - memcpy(cp.randomizer256, data->randomizer256, - sizeof(cp.randomizer256)); + memcpy(cp.rand256, data->rand256, sizeof(cp.rand256)); hci_send_cmd(hdev, HCI_OP_REMOTE_OOB_EXT_DATA_REPLY, sizeof(cp), &cp); @@ -4009,8 +4007,7 @@ static void hci_remote_oob_data_request_evt(struct hci_dev *hdev, bacpy(&cp.bdaddr, &ev->bdaddr); memcpy(cp.hash, data->hash192, sizeof(cp.hash)); - memcpy(cp.randomizer, data->randomizer192, - sizeof(cp.randomizer)); + memcpy(cp.rand, data->rand192, sizeof(cp.rand)); hci_send_cmd(hdev, HCI_OP_REMOTE_OOB_DATA_REPLY, sizeof(cp), &cp); diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 258c9826e78c..cbeef5f62f3b 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -3598,7 +3598,7 @@ static int add_remote_oob_data(struct sock *sk, struct hci_dev *hdev, } err = hci_add_remote_oob_data(hdev, &cp->addr.bdaddr, - cp->hash, cp->randomizer); + cp->hash, cp->rand); if (err < 0) status = MGMT_STATUS_FAILED; else @@ -3619,10 +3619,8 @@ static int add_remote_oob_data(struct sock *sk, struct hci_dev *hdev, } err = hci_add_remote_oob_ext_data(hdev, &cp->addr.bdaddr, - cp->hash192, - cp->randomizer192, - cp->hash256, - cp->randomizer256); + cp->hash192, cp->rand192, + cp->hash256, cp->rand256); if (err < 0) status = MGMT_STATUS_FAILED; else @@ -6771,8 +6769,8 @@ void mgmt_set_local_name_complete(struct hci_dev *hdev, u8 *name, u8 status) } void mgmt_read_local_oob_data_complete(struct hci_dev *hdev, u8 *hash192, - u8 *randomizer192, u8 *hash256, - u8 *randomizer256, u8 status) + u8 *rand192, u8 *hash256, u8 *rand256, + u8 status) { struct pending_cmd *cmd; @@ -6787,16 +6785,14 @@ void mgmt_read_local_oob_data_complete(struct hci_dev *hdev, u8 *hash192, mgmt_status(status)); } else { if (test_bit(HCI_SC_ENABLED, &hdev->dev_flags) && - hash256 && randomizer256) { + hash256 && rand256) { struct mgmt_rp_read_local_oob_ext_data rp; memcpy(rp.hash192, hash192, sizeof(rp.hash192)); - memcpy(rp.randomizer192, randomizer192, - sizeof(rp.randomizer192)); + memcpy(rp.rand192, rand192, sizeof(rp.rand192)); memcpy(rp.hash256, hash256, sizeof(rp.hash256)); - memcpy(rp.randomizer256, randomizer256, - sizeof(rp.randomizer256)); + memcpy(rp.rand256, rand256, sizeof(rp.rand256)); cmd_complete(cmd->sk, hdev->id, MGMT_OP_READ_LOCAL_OOB_DATA, 0, @@ -6805,8 +6801,7 @@ void mgmt_read_local_oob_data_complete(struct hci_dev *hdev, u8 *hash192, struct mgmt_rp_read_local_oob_data rp; memcpy(rp.hash, hash192, sizeof(rp.hash)); - memcpy(rp.randomizer, randomizer192, - sizeof(rp.randomizer)); + memcpy(rp.rand, rand192, sizeof(rp.rand)); cmd_complete(cmd->sk, hdev->id, MGMT_OP_READ_LOCAL_OOB_DATA, 0, -- cgit v1.2.3 From 76727c02c1e14a2b561b806fa1d08acc1619ad27 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Tue, 18 Nov 2014 09:00:14 +0200 Subject: Bluetooth: Call drain_workqueue() before resetting state Doing things like hci_conn_hash_flush() while holding the hdev lock is risky since its synchronous pending work cancellation could cause the L2CAP layer to try to reacquire the hdev lock. Right now there doesn't seem to be any obvious places where this would for certain happen but it's already enough to cause lockdep to start warning against the hdev and the work struct locks being taken in the "wrong" order: [ +0.000373] mgmt-tester/1603 is trying to acquire lock: [ +0.000292] ((&conn->pending_rx_work)){+.+.+.}, at: [] flush_work+0x0/0x181 [ +0.000270] but task is already holding lock: [ +0.000000] (&hdev->lock){+.+.+.}, at: [] hci_dev_do_close+0x166/0x359 [ +0.000000] which lock already depends on the new lock. [ +0.000000] the existing dependency chain (in reverse order) is: [ +0.000000] -> #1 (&hdev->lock){+.+.+.}: [ +0.000000] [] lock_acquire+0xe3/0x156 [ +0.000000] [] mutex_lock_nested+0x54/0x375 [ +0.000000] [] l2cap_recv_frame+0x293/0x1a9c [ +0.000000] [] process_pending_rx+0x50/0x5e [ +0.000000] [] process_one_work+0x21c/0x436 [ +0.000000] [] worker_thread+0x1be/0x251 [ +0.000000] [] kthread+0x94/0x99 [ +0.000000] [] ret_from_kernel_thread+0x21/0x30 [ +0.000000] -> #0 ((&conn->pending_rx_work)){+.+.+.}: [ +0.000000] [] __lock_acquire+0xa07/0xc89 [ +0.000000] [] lock_acquire+0xe3/0x156 [ +0.000000] [] flush_work+0x29/0x181 [ +0.000000] [] __cancel_work_timer+0x76/0x8f [ +0.000000] [] cancel_work_sync+0xf/0x11 [ +0.000000] [] l2cap_conn_del+0x72/0x183 [ +0.000000] [] l2cap_disconn_cfm+0x49/0x55 [ +0.000000] [] hci_conn_hash_flush+0x7a/0xc3 [ +0.000000] [] hci_dev_do_close+0x1dc/0x359 [ +0.012038] [] hci_unregister_dev+0x6e/0x1a3 [ +0.000000] [] vhci_release+0x28/0x47 [ +0.000000] [] __fput+0xd6/0x154 [ +0.000000] [] ____fput+0xd/0xf [ +0.000000] [] task_work_run+0x6b/0x8d [ +0.000000] [] do_notify_resume+0x3c/0x3f [ +0.000000] [] work_notifysig+0x29/0x31 [ +0.000000] other info that might help us debug this: [ +0.000000] Possible unsafe locking scenario: [ +0.000000] CPU0 CPU1 [ +0.000000] ---- ---- [ +0.000000] lock(&hdev->lock); [ +0.000000] lock((&conn->pending_rx_work)); [ +0.000000] lock(&hdev->lock); [ +0.000000] lock((&conn->pending_rx_work)); [ +0.000000] *** DEADLOCK *** Fully fixing this would require some quite heavy refactoring to change how the hdev lock and hci_conn instances are handled together. A simpler solution for now which this patch takes is to try ensure that the hdev workqueue is empty before proceeding with the various cleanup calls, including hci_conn_hash_flush(). Signed-off-by: Johan Hedberg Signed-off-by: Marcel Holtmann --- net/bluetooth/hci_core.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'net/bluetooth') diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index bbefb4eea36e..d786958a1dec 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -2561,6 +2561,11 @@ static int hci_dev_do_close(struct hci_dev *hdev) if (test_bit(HCI_MGMT, &hdev->dev_flags)) cancel_delayed_work_sync(&hdev->rpa_expired); + /* Avoid potential lockdep warnings from the *_flush() calls by + * ensuring the workqueue is empty up front. + */ + drain_workqueue(hdev->workqueue); + hci_dev_lock(hdev); hci_inquiry_cache_flush(hdev); hci_pend_le_actions_clear(hdev); @@ -2684,6 +2689,11 @@ int hci_dev_reset(__u16 dev) skb_queue_purge(&hdev->rx_q); skb_queue_purge(&hdev->cmd_q); + /* Avoid potential lockdep warnings from the *_flush() calls by + * ensuring the workqueue is empty up front. + */ + drain_workqueue(hdev->workqueue); + hci_dev_lock(hdev); hci_inquiry_cache_flush(hdev); hci_conn_hash_flush(hdev); -- cgit v1.2.3