[PATCH] bluetooth: fix locking in hci_sock_dev_event()
We presently use lock_sock() to acquire a lock on a socket in
hci_sock_dev_event(), but this goes BUG because lock_sock()
can sleep and we're already holding a read-write spinlock at
that point. So, we must use the non-sleeping BH version,
bh_lock_sock().
However, hci_sock_dev_event() is called from user context and
hence using simply bh_lock_sock() will deadlock against a
concurrent softirq that tries to acquire a lock on the same
socket. Hence, disabling BH's before acquiring the socket lock
and enable them afterwards, is the proper solution to fix
socket locking in hci_sock_dev_event().
Cc: David Miller <[email protected]>
Signed-off-by: Satyam Sharma <[email protected]>
Signed-off-by: Marcel Holtmann <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
---
net/bluetooth/hci_sock.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
---
diff -ruNp a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
--- a/net/bluetooth/hci_sock.c 2007-05-16 17:31:06.000000000 +0530
+++ b/net/bluetooth/hci_sock.c 2007-05-16 17:38:35.000000000 +0530
@@ -665,7 +665,8 @@ static int hci_sock_dev_event(struct not
/* Detach sockets from device */
read_lock(&hci_sk_list.lock);
sk_for_each(sk, node, &hci_sk_list.head) {
- lock_sock(sk);
+ local_bh_disable();
+ bh_lock_sock_nested(sk);
if (hci_pi(sk)->hdev == hdev) {
hci_pi(sk)->hdev = NULL;
sk->sk_err = EPIPE;
@@ -674,7 +675,8 @@ static int hci_sock_dev_event(struct not
hci_dev_put(hdev);
}
- release_sock(sk);
+ bh_unlock_sock(sk);
+ local_bh_enable();
}
read_unlock(&hci_sk_list.lock);
}
From: Satyam Sharma <[email protected]>
Date: Thu, 17 May 2007 11:13:36 +0530 (IST)
> [PATCH] bluetooth: fix locking in hci_sock_dev_event()
>
> We presently use lock_sock() to acquire a lock on a socket in
> hci_sock_dev_event(), but this goes BUG because lock_sock()
> can sleep and we're already holding a read-write spinlock at
> that point. So, we must use the non-sleeping BH version,
> bh_lock_sock().
>
> However, hci_sock_dev_event() is called from user context and
> hence using simply bh_lock_sock() will deadlock against a
> concurrent softirq that tries to acquire a lock on the same
> socket. Hence, disabling BH's before acquiring the socket lock
> and enable them afterwards, is the proper solution to fix
> socket locking in hci_sock_dev_event().
>
> Cc: David Miller <[email protected]>
> Signed-off-by: Satyam Sharma <[email protected]>
> Signed-off-by: Marcel Holtmann <[email protected]>
> Signed-off-by: Jiri Kosina <[email protected]>
Thanks I'll merge this in.
From: Satyam Sharma <[email protected]>
Date: Thu, 17 May 2007 11:13:36 +0530 (IST)
> [PATCH] bluetooth: fix locking in hci_sock_dev_event()
BTW, your email client corrupted the patch spaces and tabs
a lot, I applied your patch by hand but I will not do that
next time. Please look into getting your email client to
not corrupt the patches you send out, thank you.
On Wed, 16 May 2007, David Miller wrote:
> From: Satyam Sharma <[email protected]>
> Date: Thu, 17 May 2007 11:13:36 +0530 (IST)
>
>> [PATCH] bluetooth: fix locking in hci_sock_dev_event()
>
> BTW, your email client corrupted the patch spaces and tabs
> a lot, I applied your patch by hand but I will not do that
> next time. Please look into getting your email client to
> not corrupt the patches you send out, thank you.
Ick. That's the second time pine did that to me. Doesn't seem
to happen consistently/reproducibly, but I'll try to fix it
nonetheless, thanks.
Satyam