|
Takashi Iwai |
cc905c |
From 734bc5ff783115aa3164f4e9dd5967ae78e0a8ab Mon Sep 17 00:00:00 2001
|
|
Takashi Iwai |
cc905c |
From: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
|
|
Takashi Iwai |
cc905c |
Date: Tue, 10 Aug 2021 12:14:06 +0800
|
|
Takashi Iwai |
cc905c |
Subject: [PATCH] Bluetooth: avoid circular locks in sco_sock_connect
|
|
Takashi Iwai |
cc905c |
Git-commit: 734bc5ff783115aa3164f4e9dd5967ae78e0a8ab
|
|
Takashi Iwai |
52a00c |
Patch-mainline: v5.15-rc1
|
|
Takashi Iwai |
cc905c |
References: CVE-2021-3640 bsc#1188172
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
In a future patch, calls to bh_lock_sock in sco.c should be replaced
|
|
Takashi Iwai |
cc905c |
by lock_sock now that none of the functions are run in IRQ context.
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
However, doing so results in a circular locking dependency:
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
======================================================
|
|
Takashi Iwai |
cc905c |
Warning: possible circular locking dependency detected
|
|
Takashi Iwai |
cc905c |
5.14.0-rc4-syzkaller #0 Not tainted
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
Acked-by: Takashi Iwai <tiwai@suse.de>
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
------------------------------------------------------
|
|
Takashi Iwai |
cc905c |
syz-executor.2/14867 is trying to acquire lock:
|
|
Takashi Iwai |
cc905c |
ffff88803e3c1120 (sk_lock-AF_BLUETOOTH-BTPROTO_SCO){+.+.}-{0:0}, at:
|
|
Takashi Iwai |
cc905c |
lock_sock include/net/sock.h:1613 [inline]
|
|
Takashi Iwai |
cc905c |
ffff88803e3c1120 (sk_lock-AF_BLUETOOTH-BTPROTO_SCO){+.+.}-{0:0}, at:
|
|
Takashi Iwai |
cc905c |
sco_conn_del+0x12a/0x2a0 net/bluetooth/sco.c:191
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
but task is already holding lock:
|
|
Takashi Iwai |
cc905c |
ffffffff8d2dc7c8 (hci_cb_list_lock){+.+.}-{3:3}, at:
|
|
Takashi Iwai |
cc905c |
hci_disconn_cfm include/net/bluetooth/hci_core.h:1497 [inline]
|
|
Takashi Iwai |
cc905c |
ffffffff8d2dc7c8 (hci_cb_list_lock){+.+.}-{3:3}, at:
|
|
Takashi Iwai |
cc905c |
hci_conn_hash_flush+0xda/0x260 net/bluetooth/hci_conn.c:1608
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
which lock already depends on the new lock.
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
the existing dependency chain (in reverse order) is:
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
-> #2 (hci_cb_list_lock){+.+.}-{3:3}:
|
|
Takashi Iwai |
cc905c |
__mutex_lock_common kernel/locking/mutex.c:959 [inline]
|
|
Takashi Iwai |
cc905c |
__mutex_lock+0x12a/0x10a0 kernel/locking/mutex.c:1104
|
|
Takashi Iwai |
cc905c |
hci_connect_cfm include/net/bluetooth/hci_core.h:1482 [inline]
|
|
Takashi Iwai |
cc905c |
hci_remote_features_evt net/bluetooth/hci_event.c:3263 [inline]
|
|
Takashi Iwai |
cc905c |
hci_event_packet+0x2f4d/0x7c50 net/bluetooth/hci_event.c:6240
|
|
Takashi Iwai |
cc905c |
hci_rx_work+0x4f8/0xd30 net/bluetooth/hci_core.c:5122
|
|
Takashi Iwai |
cc905c |
process_one_work+0x98d/0x1630 kernel/workqueue.c:2276
|
|
Takashi Iwai |
cc905c |
worker_thread+0x658/0x11f0 kernel/workqueue.c:2422
|
|
Takashi Iwai |
cc905c |
kthread+0x3e5/0x4d0 kernel/kthread.c:319
|
|
Takashi Iwai |
cc905c |
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
-> #1 (&hdev->lock){+.+.}-{3:3}:
|
|
Takashi Iwai |
cc905c |
__mutex_lock_common kernel/locking/mutex.c:959 [inline]
|
|
Takashi Iwai |
cc905c |
__mutex_lock+0x12a/0x10a0 kernel/locking/mutex.c:1104
|
|
Takashi Iwai |
cc905c |
sco_connect net/bluetooth/sco.c:245 [inline]
|
|
Takashi Iwai |
cc905c |
sco_sock_connect+0x227/0xa10 net/bluetooth/sco.c:601
|
|
Takashi Iwai |
cc905c |
__sys_connect_file+0x155/0x1a0 net/socket.c:1879
|
|
Takashi Iwai |
cc905c |
__sys_connect+0x161/0x190 net/socket.c:1896
|
|
Takashi Iwai |
cc905c |
__do_sys_connect net/socket.c:1906 [inline]
|
|
Takashi Iwai |
cc905c |
__se_sys_connect net/socket.c:1903 [inline]
|
|
Takashi Iwai |
cc905c |
__x64_sys_connect+0x6f/0xb0 net/socket.c:1903
|
|
Takashi Iwai |
cc905c |
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
|
|
Takashi Iwai |
cc905c |
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
|
|
Takashi Iwai |
cc905c |
entry_SYSCALL_64_after_hwframe+0x44/0xae
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
-> #0 (sk_lock-AF_BLUETOOTH-BTPROTO_SCO){+.+.}-{0:0}:
|
|
Takashi Iwai |
cc905c |
check_prev_add kernel/locking/lockdep.c:3051 [inline]
|
|
Takashi Iwai |
cc905c |
check_prevs_add kernel/locking/lockdep.c:3174 [inline]
|
|
Takashi Iwai |
cc905c |
validate_chain kernel/locking/lockdep.c:3789 [inline]
|
|
Takashi Iwai |
cc905c |
__lock_acquire+0x2a07/0x54a0 kernel/locking/lockdep.c:5015
|
|
Takashi Iwai |
cc905c |
lock_acquire kernel/locking/lockdep.c:5625 [inline]
|
|
Takashi Iwai |
cc905c |
lock_acquire+0x1ab/0x510 kernel/locking/lockdep.c:5590
|
|
Takashi Iwai |
cc905c |
lock_sock_nested+0xca/0x120 net/core/sock.c:3170
|
|
Takashi Iwai |
cc905c |
lock_sock include/net/sock.h:1613 [inline]
|
|
Takashi Iwai |
cc905c |
sco_conn_del+0x12a/0x2a0 net/bluetooth/sco.c:191
|
|
Takashi Iwai |
cc905c |
sco_disconn_cfm+0x71/0xb0 net/bluetooth/sco.c:1202
|
|
Takashi Iwai |
cc905c |
hci_disconn_cfm include/net/bluetooth/hci_core.h:1500 [inline]
|
|
Takashi Iwai |
cc905c |
hci_conn_hash_flush+0x127/0x260 net/bluetooth/hci_conn.c:1608
|
|
Takashi Iwai |
cc905c |
hci_dev_do_close+0x528/0x1130 net/bluetooth/hci_core.c:1778
|
|
Takashi Iwai |
cc905c |
hci_unregister_dev+0x1c0/0x5a0 net/bluetooth/hci_core.c:4015
|
|
Takashi Iwai |
cc905c |
vhci_release+0x70/0xe0 drivers/bluetooth/hci_vhci.c:340
|
|
Takashi Iwai |
cc905c |
__fput+0x288/0x920 fs/file_table.c:280
|
|
Takashi Iwai |
cc905c |
task_work_run+0xdd/0x1a0 kernel/task_work.c:164
|
|
Takashi Iwai |
cc905c |
exit_task_work include/linux/task_work.h:32 [inline]
|
|
Takashi Iwai |
cc905c |
do_exit+0xbd4/0x2a60 kernel/exit.c:825
|
|
Takashi Iwai |
cc905c |
do_group_exit+0x125/0x310 kernel/exit.c:922
|
|
Takashi Iwai |
cc905c |
get_signal+0x47f/0x2160 kernel/signal.c:2808
|
|
Takashi Iwai |
cc905c |
arch_do_signal_or_restart+0x2a9/0x1c40 arch/x86/kernel/signal.c:865
|
|
Takashi Iwai |
cc905c |
handle_signal_work kernel/entry/common.c:148 [inline]
|
|
Takashi Iwai |
cc905c |
exit_to_user_mode_loop kernel/entry/common.c:172 [inline]
|
|
Takashi Iwai |
cc905c |
exit_to_user_mode_prepare+0x17d/0x290 kernel/entry/common.c:209
|
|
Takashi Iwai |
cc905c |
__syscall_exit_to_user_mode_work kernel/entry/common.c:291 [inline]
|
|
Takashi Iwai |
cc905c |
syscall_exit_to_user_mode+0x19/0x60 kernel/entry/common.c:302
|
|
Takashi Iwai |
cc905c |
ret_from_fork+0x15/0x30 arch/x86/entry/entry_64.S:288
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
other info that might help us debug this:
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
Chain exists of:
|
|
Takashi Iwai |
cc905c |
sk_lock-AF_BLUETOOTH-BTPROTO_SCO --> &hdev->lock --> hci_cb_list_lock
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
Possible unsafe locking scenario:
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
CPU0 CPU1
|
|
Takashi Iwai |
cc905c |
---- ----
|
|
Takashi Iwai |
cc905c |
lock(hci_cb_list_lock);
|
|
Takashi Iwai |
cc905c |
lock(&hdev->lock);
|
|
Takashi Iwai |
cc905c |
lock(hci_cb_list_lock);
|
|
Takashi Iwai |
cc905c |
lock(sk_lock-AF_BLUETOOTH-BTPROTO_SCO);
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
*** DEADLOCK ***
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
The issue is that the lock hierarchy should go from &hdev->lock -->
|
|
Takashi Iwai |
cc905c |
hci_cb_list_lock --> sk_lock-AF_BLUETOOTH-BTPROTO_SCO. For example,
|
|
Takashi Iwai |
cc905c |
one such call trace is:
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
hci_dev_do_close():
|
|
Takashi Iwai |
cc905c |
hci_dev_lock();
|
|
Takashi Iwai |
cc905c |
hci_conn_hash_flush():
|
|
Takashi Iwai |
cc905c |
hci_disconn_cfm():
|
|
Takashi Iwai |
cc905c |
mutex_lock(&hci_cb_list_lock);
|
|
Takashi Iwai |
cc905c |
sco_disconn_cfm():
|
|
Takashi Iwai |
cc905c |
sco_conn_del():
|
|
Takashi Iwai |
cc905c |
lock_sock(sk);
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
However, in sco_sock_connect, we call lock_sock before calling
|
|
Takashi Iwai |
cc905c |
hci_dev_lock inside sco_connect, thus inverting the lock hierarchy.
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
We fix this by pulling the call to hci_dev_lock out from sco_connect.
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
|
|
Takashi Iwai |
cc905c |
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
|
|
Takashi Iwai |
cc905c |
---
|
|
Takashi Iwai |
cc905c |
net/bluetooth/sco.c | 39 ++++++++++++++++-----------------------
|
|
Takashi Iwai |
cc905c |
1 file changed, 16 insertions(+), 23 deletions(-)
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
|
|
Takashi Iwai |
cc905c |
index 62e638f971a9..94a3aa686556 100644
|
|
Takashi Iwai |
cc905c |
--- a/net/bluetooth/sco.c
|
|
Takashi Iwai |
cc905c |
+++ b/net/bluetooth/sco.c
|
|
Takashi Iwai |
cc905c |
@@ -237,44 +237,32 @@ static int sco_chan_add(struct sco_conn *conn, struct sock *sk,
|
|
Takashi Iwai |
cc905c |
return err;
|
|
Takashi Iwai |
cc905c |
}
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
-static int sco_connect(struct sock *sk)
|
|
Takashi Iwai |
cc905c |
+static int sco_connect(struct hci_dev *hdev, struct sock *sk)
|
|
Takashi Iwai |
cc905c |
{
|
|
Takashi Iwai |
cc905c |
struct sco_conn *conn;
|
|
Takashi Iwai |
cc905c |
struct hci_conn *hcon;
|
|
Takashi Iwai |
cc905c |
- struct hci_dev *hdev;
|
|
Takashi Iwai |
cc905c |
int err, type;
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
BT_DBG("%pMR -> %pMR", &sco_pi(sk)->src, &sco_pi(sk)->dst);
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
- hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
|
|
Takashi Iwai |
cc905c |
- if (!hdev)
|
|
Takashi Iwai |
cc905c |
- return -EHOSTUNREACH;
|
|
Takashi Iwai |
cc905c |
-
|
|
Takashi Iwai |
cc905c |
- hci_dev_lock(hdev);
|
|
Takashi Iwai |
cc905c |
-
|
|
Takashi Iwai |
cc905c |
if (lmp_esco_capable(hdev) && !disable_esco)
|
|
Takashi Iwai |
cc905c |
type = ESCO_LINK;
|
|
Takashi Iwai |
cc905c |
else
|
|
Takashi Iwai |
cc905c |
type = SCO_LINK;
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
if (sco_pi(sk)->setting == BT_VOICE_TRANSPARENT &&
|
|
Takashi Iwai |
cc905c |
- (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev))) {
|
|
Takashi Iwai |
cc905c |
- err = -EOPNOTSUPP;
|
|
Takashi Iwai |
cc905c |
- goto done;
|
|
Takashi Iwai |
cc905c |
- }
|
|
Takashi Iwai |
cc905c |
+ (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev)))
|
|
Takashi Iwai |
cc905c |
+ return -EOPNOTSUPP;
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst,
|
|
Takashi Iwai |
cc905c |
sco_pi(sk)->setting);
|
|
Takashi Iwai |
cc905c |
- if (IS_ERR(hcon)) {
|
|
Takashi Iwai |
cc905c |
- err = PTR_ERR(hcon);
|
|
Takashi Iwai |
cc905c |
- goto done;
|
|
Takashi Iwai |
cc905c |
- }
|
|
Takashi Iwai |
cc905c |
+ if (IS_ERR(hcon))
|
|
Takashi Iwai |
cc905c |
+ return PTR_ERR(hcon);
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
conn = sco_conn_add(hcon);
|
|
Takashi Iwai |
cc905c |
if (!conn) {
|
|
Takashi Iwai |
cc905c |
hci_conn_drop(hcon);
|
|
Takashi Iwai |
cc905c |
- err = -ENOMEM;
|
|
Takashi Iwai |
cc905c |
- goto done;
|
|
Takashi Iwai |
cc905c |
+ return -ENOMEM;
|
|
Takashi Iwai |
cc905c |
}
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
/* Update source addr of the socket */
|
|
Takashi Iwai |
cc905c |
@@ -282,7 +270,7 @@ static int sco_connect(struct sock *sk)
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
err = sco_chan_add(conn, sk, NULL);
|
|
Takashi Iwai |
cc905c |
if (err)
|
|
Takashi Iwai |
cc905c |
- goto done;
|
|
Takashi Iwai |
cc905c |
+ return err;
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
if (hcon->state == BT_CONNECTED) {
|
|
Takashi Iwai |
cc905c |
sco_sock_clear_timer(sk);
|
|
Takashi Iwai |
cc905c |
@@ -292,9 +280,6 @@ static int sco_connect(struct sock *sk)
|
|
Takashi Iwai |
cc905c |
sco_sock_set_timer(sk, sk->sk_sndtimeo);
|
|
Takashi Iwai |
cc905c |
}
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
-done:
|
|
Takashi Iwai |
cc905c |
- hci_dev_unlock(hdev);
|
|
Takashi Iwai |
cc905c |
- hci_dev_put(hdev);
|
|
Takashi Iwai |
cc905c |
return err;
|
|
Takashi Iwai |
cc905c |
}
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
@@ -589,6 +574,7 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
|
|
Takashi Iwai |
cc905c |
{
|
|
Takashi Iwai |
cc905c |
struct sockaddr_sco *sa = (struct sockaddr_sco *) addr;
|
|
Takashi Iwai |
cc905c |
struct sock *sk = sock->sk;
|
|
Takashi Iwai |
cc905c |
+ struct hci_dev *hdev;
|
|
Takashi Iwai |
cc905c |
int err;
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
BT_DBG("sk %p", sk);
|
|
Takashi Iwai |
cc905c |
@@ -603,12 +589,19 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
|
|
Takashi Iwai |
cc905c |
if (sk->sk_type != SOCK_SEQPACKET)
|
|
Takashi Iwai |
cc905c |
return -EINVAL;
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
+ hdev = hci_get_route(&sa->sco_bdaddr, &sco_pi(sk)->src, BDADDR_BREDR);
|
|
Takashi Iwai |
cc905c |
+ if (!hdev)
|
|
Takashi Iwai |
cc905c |
+ return -EHOSTUNREACH;
|
|
Takashi Iwai |
cc905c |
+ hci_dev_lock(hdev);
|
|
Takashi Iwai |
cc905c |
+
|
|
Takashi Iwai |
cc905c |
lock_sock(sk);
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
/* Set destination address and psm */
|
|
Takashi Iwai |
cc905c |
bacpy(&sco_pi(sk)->dst, &sa->sco_bdaddr);
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
- err = sco_connect(sk);
|
|
Takashi Iwai |
cc905c |
+ err = sco_connect(hdev, sk);
|
|
Takashi Iwai |
cc905c |
+ hci_dev_unlock(hdev);
|
|
Takashi Iwai |
cc905c |
+ hci_dev_put(hdev);
|
|
Takashi Iwai |
cc905c |
if (err)
|
|
Takashi Iwai |
cc905c |
goto done;
|
|
Takashi Iwai |
cc905c |
|
|
Takashi Iwai |
cc905c |
--
|
|
Takashi Iwai |
cc905c |
2.26.2
|
|
Takashi Iwai |
cc905c |
|