[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 7 Jul 2021 18:43:36 +0900
From: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To: Marcel Holtmann <marcel@...tmann.org>,
Johan Hedberg <johan.hedberg@...il.com>,
Luiz Augusto von Dentz <luiz.dentz@...il.com>,
linux-bluetooth@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Lin Ma <linma@....edu.cn>,
netdev@...r.kernel.org
Subject: [PATCH v2] Bluetooth: call lock_sock() outside of spinlock section
syzbot is hitting might_sleep() warning at hci_sock_dev_event() due to
calling lock_sock() with rw spinlock held [1]. Defer calling lock_sock()
via sock_hold().
Link: https://syzkaller.appspot.com/bug?extid=a5df189917e79d5e59c9 [1]
Reported-by: syzbot <syzbot+a5df189917e79d5e59c9@...kaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Tested-by: syzbot <syzbot+a5df189917e79d5e59c9@...kaller.appspotmail.com>
Fixes: e305509e678b3a4a ("Bluetooth: use correct lock to prevent UAF of hdev object")
---
Changes in v2:
Take hci_sk_list.lock for write in case bt_sock_unlink() is called after
sk_hashed(sk) test, and defer hci_dev_put(hdev) till schedulable context.
net/bluetooth/hci_sock.c | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index b04a5a02ecf3..d8e1ac1ae10d 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -758,20 +758,46 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event)
if (event == HCI_DEV_UNREG) {
struct sock *sk;
+ bool put_dev;
+restart:
+ put_dev = false;
/* Detach sockets from device */
read_lock(&hci_sk_list.lock);
sk_for_each(sk, &hci_sk_list.head) {
+ /* hci_sk_list.lock is preventing hci_sock_release()
+ * from calling bt_sock_unlink().
+ */
+ if (hci_pi(sk)->hdev != hdev || sk_unhashed(sk))
+ continue;
+ /* Take a ref because we can't call lock_sock() with
+ * hci_sk_list.lock held.
+ */
+ sock_hold(sk);
+ read_unlock(&hci_sk_list.lock);
lock_sock(sk);
- if (hci_pi(sk)->hdev == hdev) {
+ /* Since hci_sock_release() might have already called
+ * bt_sock_unlink() while waiting for lock_sock(),
+ * use sk_hashed(sk) for checking that bt_sock_unlink()
+ * is not yet called.
+ */
+ write_lock(&hci_sk_list.lock);
+ if (sk_hashed(sk) && hci_pi(sk)->hdev == hdev) {
hci_pi(sk)->hdev = NULL;
sk->sk_err = EPIPE;
sk->sk_state = BT_OPEN;
sk->sk_state_change(sk);
-
- hci_dev_put(hdev);
+ put_dev = true;
}
+ write_unlock(&hci_sk_list.lock);
release_sock(sk);
+ sock_put(sk);
+ if (put_dev)
+ hci_dev_put(hdev);
+ /* Restarting is safe, for hci_pi(sk)->hdev != hdev if
+ * condition met and sk_unhashed(sk) == true otherwise.
+ */
+ goto restart;
}
read_unlock(&hci_sk_list.lock);
}
--
2.18.4
Powered by blists - more mailing lists