From 88149db4948ef90cf6220d76e34955e46c2ff9f9 Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Mon, 26 Sep 2011 14:19:47 +0200 Subject: [PATCH 1/7] Bluetooth: rfcomm: Fix sleep in invalid context in rfcomm_security_cfm This was triggered by turning off encryption on ACL link when rfcomm was using high security. rfcomm_security_cfm (which is called from rx task) was closing DLC and this involves sending disconnect message (and locking socket). Move closing DLC to rfcomm_process_dlcs and only flag DLC for closure in rfcomm_security_cfm. BUG: sleeping function called from invalid context at net/core/sock.c:2032 in_atomic(): 1, irqs_disabled(): 0, pid: 1788, name: kworker/0:3 [] (unwind_backtrace+0x0/0x108) from [] (dump_stack+0x20/0x24) [] (dump_stack+0x20/0x24) from [] (__might_sleep+0x110/0x12c) [] (__might_sleep+0x110/0x12c) from [] (lock_sock_nested+0x2c/0x64) [] (lock_sock_nested+0x2c/0x64) from [] (l2cap_sock_sendmsg+0x58/0xcc) [] (l2cap_sock_sendmsg+0x58/0xcc) from [] (sock_sendmsg+0xb0/0xd0) [] (sock_sendmsg+0xb0/0xd0) from [] (kernel_sendmsg+0x3c/0x44) [] (kernel_sendmsg+0x3c/0x44) from [] (rfcomm_send_frame+0x50/0x58) [] (rfcomm_send_frame+0x50/0x58) from [] (rfcomm_send_disc+0x78/0x80) [] (rfcomm_send_disc+0x78/0x80) from [] (__rfcomm_dlc_close+0x2d0/0x2fc) [] (__rfcomm_dlc_close+0x2d0/0x2fc) from [] (rfcomm_security_cfm+0x140/0x1e0) [] (rfcomm_security_cfm+0x140/0x1e0) from [] (hci_event_packet+0x1ce8/0x4d84) [] (hci_event_packet+0x1ce8/0x4d84) from [] (hci_rx_task+0x1d0/0x2d0) [] (hci_rx_task+0x1d0/0x2d0) from [] (tasklet_action+0x138/0x1e4) [] (tasklet_action+0x138/0x1e4) from [] (__do_softirq+0xcc/0x274) [] (__do_softirq+0xcc/0x274) from [] (do_softirq+0x60/0x6c) [] (do_softirq+0x60/0x6c) from [] (local_bh_enable_ip+0xc8/0xd4) [] (local_bh_enable_ip+0xc8/0xd4) from [] (_raw_spin_unlock_bh+0x48/0x4c) [] (_raw_spin_unlock_bh+0x48/0x4c) from [] (data_from_chip+0xf4/0xaec) [] (data_from_chip+0xf4/0xaec) from [] (send_skb_to_core+0x40/0x178) [] (send_skb_to_core+0x40/0x178) from [] (cg2900_hu_receive+0x15c/0x2d0) [] (cg2900_hu_receive+0x15c/0x2d0) from [] (hci_uart_tty_receive+0x74/0xa0) [] (hci_uart_tty_receive+0x74/0xa0) from [] (flush_to_ldisc+0x188/0x198) [] (flush_to_ldisc+0x188/0x198) from [] (process_one_work+0x144/0x4b8) [] (process_one_work+0x144/0x4b8) from [] (worker_thread+0x198/0x468) [] (worker_thread+0x198/0x468) from [] (kthread+0x98/0xa0) [] (kthread+0x98/0xa0) from [] (kernel_thread_exit+0x0/0x8) Signed-off-by: Szymon Janc Signed-off-by: Gustavo F. Padovan --- include/net/bluetooth/rfcomm.h | 1 + net/bluetooth/rfcomm/core.c | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h index d5eee2093b1e..e2e3ecad1008 100644 --- a/include/net/bluetooth/rfcomm.h +++ b/include/net/bluetooth/rfcomm.h @@ -211,6 +211,7 @@ struct rfcomm_dlc { #define RFCOMM_AUTH_ACCEPT 6 #define RFCOMM_AUTH_REJECT 7 #define RFCOMM_DEFER_SETUP 8 +#define RFCOMM_ENC_DROP 9 /* Scheduling flags and events */ #define RFCOMM_SCHED_WAKEUP 31 diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c index 5ba3f6df665c..71ef2581c60f 100644 --- a/net/bluetooth/rfcomm/core.c +++ b/net/bluetooth/rfcomm/core.c @@ -1802,6 +1802,11 @@ static inline void rfcomm_process_dlcs(struct rfcomm_session *s) continue; } + if (test_bit(RFCOMM_ENC_DROP, &d->flags)) { + __rfcomm_dlc_close(d, ECONNREFUSED); + continue; + } + if (test_and_clear_bit(RFCOMM_AUTH_ACCEPT, &d->flags)) { rfcomm_dlc_clear_timer(d); if (d->out) { @@ -2074,7 +2079,7 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) if (test_and_clear_bit(RFCOMM_SEC_PENDING, &d->flags)) { rfcomm_dlc_clear_timer(d); if (status || encrypt == 0x00) { - __rfcomm_dlc_close(d, ECONNREFUSED); + set_bit(RFCOMM_ENC_DROP, &d->flags); continue; } } @@ -2085,7 +2090,7 @@ static void rfcomm_security_cfm(struct hci_conn *conn, u8 status, u8 encrypt) rfcomm_dlc_set_timer(d, RFCOMM_AUTH_TIMEOUT); continue; } else if (d->sec_level == BT_SECURITY_HIGH) { - __rfcomm_dlc_close(d, ECONNREFUSED); + set_bit(RFCOMM_ENC_DROP, &d->flags); continue; } } From e1b6eb3ccb0c2a34302a9fd87dd15d7b86337f23 Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Mon, 17 Oct 2011 23:05:49 +0200 Subject: [PATCH 2/7] Bluetooth: Increase HCI reset timeout in hci_dev_do_close I've noticed that my CSR usb dongle was not working if it was plugged in when PC was booting. It looks like I get two HCI reset command complete events (see hcidump logs below). The root cause is reset called from off_timer. Timeout for this reset to complete is set to 250ms and my bt dongle requires more time for replying with command complete event. After that, chip seems to reply with reset command complete event for next non-reset command. Attached patch increase mentioned timeout to HCI_INIT_TIMEOUT, this value is already used for timeouting hci_reset_req in hci_dev_reset(). This might also be related to BT not working after suspend that was reported here some time ago. Hcidump log: 2011-09-12 23:13:27.379465 < HCI Command: Reset (0x03|0x0003) plen 0 2011-09-12 23:13:27.380797 > HCI Event: Command Complete (0x0e) plen 4 Reset (0x03|0x0003) ncmd 1 status 0x00 2011-09-12 23:13:27.380859 < HCI Command: Read Local Supported Features (0x04|0x000 3) plen 0 2011-09-12 23:13:27.760789 > HCI Event: Command Complete (0x0e) plen 4 Reset (0x03|0x0003) ncmd 1 status 0x00 2011-09-12 23:13:27.760831 < HCI Command: Read Local Version Information (0x04|0x00 01) plen 0 2011-09-12 23:13:27.764780 > HCI Event: Command Complete (0x0e) plen 12 Read Local Version Information (0x04|0x0001) ncmd 1 status 0x00 HCI Version: 1.1 (0x1) HCI Revision: 0x36f LMP Version: 1.1 (0x1) LMP Subversion: 0x36f Manufacturer: Cambridge Silicon Radio (10) Signed-off-by: Szymon Janc Signed-off-by: Gustavo F. Padovan --- net/bluetooth/hci_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 56943add45cc..44fb4a7e5c7d 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -613,7 +613,7 @@ static int hci_dev_do_close(struct hci_dev *hdev) if (!test_bit(HCI_RAW, &hdev->flags)) { set_bit(HCI_INIT, &hdev->flags); __hci_request(hdev, hci_reset_req, 0, - msecs_to_jiffies(250)); + msecs_to_jiffies(HCI_INIT_TIMEOUT)); clear_bit(HCI_INIT, &hdev->flags); } From dafbde395ed560ddc3695df40f61d91c47433228 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 24 Oct 2011 22:36:26 +0200 Subject: [PATCH 3/7] Bluetooth: Set HCI_MGMT flag only in read_controller_info The HCI_MGMT flag should only be set when user space requests the full controller information. This way we avoid potential issues with setting change events ariving before the actual read_controller_info command finishes. Signed-off-by: Johan Hedberg Signed-off-by: Gustavo F. Padovan --- net/bluetooth/mgmt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 53e109eb043e..99251d19bf6f 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -147,8 +147,6 @@ static int read_index_list(struct sock *sk) hci_del_off_timer(d); - set_bit(HCI_MGMT, &d->flags); - if (test_bit(HCI_SETUP, &d->flags)) continue; From 0ba22bb258341783939ed07bf7e7a9d92f3b0edf Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 25 Oct 2011 12:09:52 +0200 Subject: [PATCH 4/7] Bluetooth: ath3k: Use GFP_KERNEL instead of GFP_ATOMIC We are allowed to sleep here so no need to use GFP_ATOMIC. The caller (ath3k_probe) calls request_firmware() which definitely sleeps. Hence, we should avoid using GFP_ATOMIC. Signed-off-by: David Herrmann Signed-off-by: Gustavo F. Padovan --- drivers/bluetooth/ath3k.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c index db7cb8111fbe..106beb194f3c 100644 --- a/drivers/bluetooth/ath3k.c +++ b/drivers/bluetooth/ath3k.c @@ -105,7 +105,7 @@ static int ath3k_load_firmware(struct usb_device *udev, pipe = usb_sndctrlpipe(udev, 0); - send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC); + send_buf = kmalloc(BULK_SIZE, GFP_KERNEL); if (!send_buf) { BT_ERR("Can't allocate memory chunk for firmware"); return -ENOMEM; @@ -176,7 +176,7 @@ static int ath3k_load_fwfile(struct usb_device *udev, count = firmware->size; - send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC); + send_buf = kmalloc(BULK_SIZE, GFP_KERNEL); if (!send_buf) { BT_ERR("Can't allocate memory chunk for firmware"); return -ENOMEM; From fc501ad7a10a356819505b1e526079d47fdebc2c Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 26 Oct 2011 11:13:13 +0200 Subject: [PATCH 5/7] Bluetooth: bcm203x: Fix race condition on disconnect When disconnecting a bcm203x device we kill and destroy the usb-urb, however, there might still be a pending work-structure which resubmits the now invalid urb. To avoid this race condition, we simply set a shutdown-flag and synchronously kill the worker first. This also adds a comment to all schedule_work()s, as it is really not clear that they are used as replacement for short timers (which can be seen in the git history). Signed-off-by: David Herrmann Acked-by: Marcel Holtmann Signed-off-by: Gustavo F. Padovan --- drivers/bluetooth/bcm203x.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/bluetooth/bcm203x.c b/drivers/bluetooth/bcm203x.c index 8b1b643a519b..ec743c2ddf9d 100644 --- a/drivers/bluetooth/bcm203x.c +++ b/drivers/bluetooth/bcm203x.c @@ -24,6 +24,7 @@ #include +#include #include #include #include @@ -65,6 +66,7 @@ struct bcm203x_data { unsigned long state; struct work_struct work; + atomic_t shutdown; struct urb *urb; unsigned char *buffer; @@ -97,6 +99,7 @@ static void bcm203x_complete(struct urb *urb) data->state = BCM203X_SELECT_MEMORY; + /* use workqueue to have a small delay */ schedule_work(&data->work); break; @@ -155,6 +158,9 @@ static void bcm203x_work(struct work_struct *work) struct bcm203x_data *data = container_of(work, struct bcm203x_data, work); + if (atomic_read(&data->shutdown)) + return; + if (usb_submit_urb(data->urb, GFP_ATOMIC) < 0) BT_ERR("Can't submit URB"); } @@ -243,6 +249,7 @@ static int bcm203x_probe(struct usb_interface *intf, const struct usb_device_id usb_set_intfdata(intf, data); + /* use workqueue to have a small delay */ schedule_work(&data->work); return 0; @@ -254,6 +261,9 @@ static void bcm203x_disconnect(struct usb_interface *intf) BT_DBG("intf %p", intf); + atomic_inc(&data->shutdown); + cancel_work_sync(&data->work); + usb_kill_urb(data->urb); usb_set_intfdata(intf, NULL); From b91a4e3e3a16085623d47f03b338d9b74954ac67 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 25 Oct 2011 21:13:36 +0200 Subject: [PATCH 6/7] Bluetooth: bcm203x: Use GFP_KERNEL in workqueue A workqueue is allowed to sleep so we can safely use GFP_KERNEL instead of GFP_ATOMIC. This is still legacy code when the driver used timer BHs and not a worqueue. Signed-off-by: David Herrmann Acked-by: Marcel Holtmann Signed-off-by: Gustavo F. Padovan --- drivers/bluetooth/bcm203x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/bluetooth/bcm203x.c b/drivers/bluetooth/bcm203x.c index ec743c2ddf9d..54952ab800b8 100644 --- a/drivers/bluetooth/bcm203x.c +++ b/drivers/bluetooth/bcm203x.c @@ -161,7 +161,7 @@ static void bcm203x_work(struct work_struct *work) if (atomic_read(&data->shutdown)) return; - if (usb_submit_urb(data->urb, GFP_ATOMIC) < 0) + if (usb_submit_urb(data->urb, GFP_KERNEL) < 0) BT_ERR("Can't submit URB"); } From 6b441fab28ea1cbbf3da75dcd1e7438e6cba704c Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 26 Oct 2011 11:22:46 +0200 Subject: [PATCH 7/7] Bluetooth: bfusb: Fix error path on firmware load When loading the usb-configuration we do not signal the end of configuration on memory allocation error. This patch moves the memory allocation to the top so every error path uses "goto error" now to correctly send the usb-ctrl message when detecting some error. This also replaces GFP_ATOMIC with GFP_KERNEL as we are allowed to sleep here. Signed-off-by: David Herrmann Acked-by: Marcel Holtmann Signed-off-by: Gustavo F. Padovan --- drivers/bluetooth/bfusb.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/bluetooth/bfusb.c b/drivers/bluetooth/bfusb.c index 005919ab043c..61b591470a90 100644 --- a/drivers/bluetooth/bfusb.c +++ b/drivers/bluetooth/bfusb.c @@ -568,22 +568,23 @@ static int bfusb_load_firmware(struct bfusb_data *data, BT_INFO("BlueFRITZ! USB loading firmware"); + buf = kmalloc(BFUSB_MAX_BLOCK_SIZE + 3, GFP_KERNEL); + if (!buf) { + BT_ERR("Can't allocate memory chunk for firmware"); + return -ENOMEM; + } + pipe = usb_sndctrlpipe(data->udev, 0); if (usb_control_msg(data->udev, pipe, USB_REQ_SET_CONFIGURATION, 0, 1, 0, NULL, 0, USB_CTRL_SET_TIMEOUT) < 0) { BT_ERR("Can't change to loading configuration"); + kfree(buf); return -EBUSY; } data->udev->toggle[0] = data->udev->toggle[1] = 0; - buf = kmalloc(BFUSB_MAX_BLOCK_SIZE + 3, GFP_ATOMIC); - if (!buf) { - BT_ERR("Can't allocate memory chunk for firmware"); - return -ENOMEM; - } - pipe = usb_sndbulkpipe(data->udev, data->bulk_out_ep); while (count) {