From 8aded7110a5625bc00aef05e94dd4b1a9cf3605f Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Thu, 7 Jul 2011 10:30:35 -0300 Subject: [PATCH] Bluetooth: Fix potential deadlock in hci_core Since hdev->lock may be acquired by threads runnning in interrupt context, all threads running in process context should disable local bottom halve before locking hdev->lock. This can be done by using hci_dev_lock_bh macro. This way, we avoid potencial deadlocks like this one reported by CONFIG_PROVE_LOCKING=y. [ 304.788780] ================================= [ 304.789686] [ INFO: inconsistent lock state ] [ 304.789686] 2.6.39+ #1 [ 304.789686] --------------------------------- [ 304.789686] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. [ 304.789686] ksoftirqd/0/3 [HC0[0]:SC1[1]:HE1:SE0] takes: [ 304.789686] (&(&hdev->lock)->rlock){+.?...}, at: [] hci_conn_check_pending+0x38/0x76 [bluetooth] [ 304.789686] {SOFTIRQ-ON-W} state was registered at: [ 304.789686] [] __lock_acquire+0x347/0xd52 [ 304.789686] [] lock_acquire+0x8a/0xa7 [ 304.789686] [] _raw_spin_lock+0x2c/0x3b [ 304.789686] [] hci_blacklist_del+0x1f/0x8a [bluetooth] [ 304.789686] [] hci_sock_ioctl+0x2d9/0x314 [bluetooth] [ 304.789686] [] sock_ioctl+0x1f2/0x214 [ 304.789686] [] do_vfs_ioctl+0x46c/0x4ad [ 304.789686] [] sys_ioctl+0x42/0x65 [ 304.789686] [] system_call_fastpath+0x16/0x1b [ 304.789686] irq event stamp: 9768 [ 304.789686] hardirqs last enabled at (9768): [] restore_args+0x0/0x30 [ 304.789686] hardirqs last disabled at (9767): [] save_args+0x6a/0x70 [ 304.789686] softirqs last enabled at (9726): [] __do_softirq+0x129/0x13f [ 304.789686] softirqs last disabled at (9739): [] run_ksoftirqd+0x82/0x133 [ 304.789686] [ 304.789686] other info that might help us debug this: [ 304.789686] Possible unsafe locking scenario: [ 304.789686] [ 304.789686] CPU0 [ 304.789686] ---- [ 304.789686] lock(&(&hdev->lock)->rlock); [ 304.789686] [ 304.789686] lock(&(&hdev->lock)->rlock); [ 304.789686] [ 304.789686] *** DEADLOCK *** [ 304.789686] [ 304.789686] 1 lock held by ksoftirqd/0/3: [ 304.789686] #0: (hci_task_lock){++.-..}, at: [] hci_rx_task+0x49/0x2f3 [bluetooth] [ 304.789686] [ 304.789686] stack backtrace: [ 304.789686] Pid: 3, comm: ksoftirqd/0 Not tainted 2.6.39+ #1 [ 304.789686] Call Trace: [ 304.789686] [] print_usage_bug+0x1e7/0x1f8 [ 304.789686] [] ? save_stack_trace+0x27/0x44 [ 304.789686] [] ? print_irq_inversion_bug.part.26+0x19a/0x19a [ 304.789686] [] mark_lock+0x106/0x258 [ 304.789686] [] ? retint_restore_args+0x13/0x13 [ 304.789686] [] __lock_acquire+0x2d3/0xd52 [ 304.789686] [] ? vprintk+0x3ab/0x3d7 [ 304.789686] [] ? printk+0x3c/0x3e [ 304.789686] [] lock_acquire+0x8a/0xa7 [ 304.789686] [] ? hci_conn_check_pending+0x38/0x76 [bluetooth] [ 304.789686] [] ? __dynamic_pr_debug+0x10c/0x11a [ 304.789686] [] _raw_spin_lock+0x2c/0x3b [ 304.789686] [] ? hci_conn_check_pending+0x38/0x76 [bluetooth] [ 304.789686] [] hci_conn_check_pending+0x38/0x76 [bluetooth] [ 304.789686] [] hci_event_packet+0x38e/0x3e12 [bluetooth] [ 304.789686] [] ? lock_release+0x16c/0x179 [ 304.789686] [] ? _raw_read_unlock+0x23/0x27 [ 304.789686] [] ? hci_send_to_sock+0x179/0x188 [bluetooth] [ 304.789686] [] hci_rx_task+0xc8/0x2f3 [bluetooth] [ 304.789686] [] tasklet_action+0x87/0xe6 [ 304.789686] [] __do_softirq+0x9f/0x13f [ 304.789686] [] run_ksoftirqd+0x82/0x133 [ 304.789686] [] ? __do_softirq+0x13f/0x13f [ 304.789686] [] kthread+0x7f/0x87 [ 304.789686] [] kernel_thread_helper+0x4/0x10 [ 304.789686] [] ? retint_restore_args+0x13/0x13 [ 304.789686] [] ? __init_kthread_worker+0x53/0x53 [ 304.789686] [] ? gs_change+0x13/0x13 Signed-off-by: Andre Guedes Signed-off-by: Gustavo F. Padovan --- net/bluetooth/hci_core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 2709335..7ba1ca1 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1244,7 +1244,7 @@ int hci_blacklist_add(struct hci_dev *hdev, bdaddr_t *bdaddr) if (bacmp(bdaddr, BDADDR_ANY) == 0) return -EBADF; - hci_dev_lock(hdev); + hci_dev_lock_bh(hdev); if (hci_blacklist_lookup(hdev, bdaddr)) { err = -EEXIST; @@ -1264,7 +1264,7 @@ int hci_blacklist_add(struct hci_dev *hdev, bdaddr_t *bdaddr) err = 0; err: - hci_dev_unlock(hdev); + hci_dev_unlock_bh(hdev); return err; } @@ -1273,7 +1273,7 @@ int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr) struct bdaddr_list *entry; int err = 0; - hci_dev_lock(hdev); + hci_dev_lock_bh(hdev); if (bacmp(bdaddr, BDADDR_ANY) == 0) { hci_blacklist_clear(hdev); @@ -1290,7 +1290,7 @@ int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr) kfree(entry); done: - hci_dev_unlock(hdev); + hci_dev_unlock_bh(hdev); return err; } -- 2.7.4