Bluetooth: ISO: Fix data-race on iso_pi fields in hci_get_route calls

iso_connect_bis(), iso_connect_cis(), iso_listen_bis(), and
iso_conn_big_sync() call hci_get_route() using iso_pi(sk)->dst,
iso_pi(sk)->src, and iso_pi(sk)->src_type without holding lock_sock().

These fields may be modified concurrently by connect() or setsockopt()
on the same socket, resulting in data-races reported by KCSAN.

Fix this by snapshotting the required fields under lock_sock() before
calling hci_get_route().

BUG: KCSAN: data-race in memcmp+0x45/0xb0

race at unknown origin, with read to 0xffff8880122135cf of 1 bytes
by task 333 on cpu 1:
 memcmp+0x45/0xb0
 hci_get_route+0x27e/0x490
 iso_connect_cis+0x4c/0xa10
 iso_sock_connect+0x60e/0xb30
 __sys_connect_file+0xbd/0xe0
 __sys_connect+0xe0/0x110
 __x64_sys_connect+0x40/0x50
 x64_sys_call+0xcad/0x1c60
 do_syscall_64+0x133/0x590
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fixes: 241f51931c35 ("Bluetooth: ISO: Avoid circular locking dependency")
Signed-off-by: SeungJu Cheon <suunj1331@gmail.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index c21ed2bb3cf8f9b9d77aedcaa9a2b153475846c5..3abd8111dda83bc761ecf40971901055de63ef12 100644 (file)
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -337,12 +337,20 @@ static int iso_connect_bis(struct sock *sk)
        struct iso_conn *conn;
        struct hci_conn *hcon;
        struct hci_dev  *hdev;
+       bdaddr_t src, dst;
+       u8 src_type, bc_sid;
        int err;
 
-       BT_DBG("%pMR (SID 0x%2.2x)", &iso_pi(sk)->src, iso_pi(sk)->bc_sid);
+       lock_sock(sk);
+       bacpy(&src, &iso_pi(sk)->src);
+       bacpy(&dst, &iso_pi(sk)->dst);
+       src_type = iso_pi(sk)->src_type;
+       bc_sid = iso_pi(sk)->bc_sid;
+       release_sock(sk);
 
-       hdev = hci_get_route(&iso_pi(sk)->dst, &iso_pi(sk)->src,
-                            iso_pi(sk)->src_type);
+       BT_DBG("%pMR (SID 0x%2.2x)", &src, bc_sid);
+
+       hdev = hci_get_route(&dst, &src, src_type);
        if (!hdev)
                return -EHOSTUNREACH;
 
@@ -430,12 +438,19 @@ static int iso_connect_cis(struct sock *sk)
        struct iso_conn *conn;
        struct hci_conn *hcon;
        struct hci_dev  *hdev;
+       bdaddr_t src, dst;
+       u8 src_type;
        int err;
 
-       BT_DBG("%pMR -> %pMR", &iso_pi(sk)->src, &iso_pi(sk)->dst);
+       lock_sock(sk);
+       bacpy(&src, &iso_pi(sk)->src);
+       bacpy(&dst, &iso_pi(sk)->dst);
+       src_type = iso_pi(sk)->src_type;
+       release_sock(sk);
+
+       BT_DBG("%pMR -> %pMR", &src, &dst);
 
-       hdev = hci_get_route(&iso_pi(sk)->dst, &iso_pi(sk)->src,
-                            iso_pi(sk)->src_type);
+       hdev = hci_get_route(&dst, &src, src_type);
        if (!hdev)
                return -EHOSTUNREACH;
 
@@ -1212,18 +1227,25 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr_unsized *addr,
 
 static int iso_listen_bis(struct sock *sk)
 {
-       struct hci_dev *hdev;
-       int err = 0;
        struct iso_conn *conn;
        struct hci_conn *hcon;
+       struct hci_dev *hdev;
+       bdaddr_t src, dst;
+       u8 src_type, bc_sid;
+       int err = 0;
+
+       lock_sock(sk);
+       bacpy(&src, &iso_pi(sk)->src);
+       bacpy(&dst, &iso_pi(sk)->dst);
+       src_type = iso_pi(sk)->src_type;
+       bc_sid = iso_pi(sk)->bc_sid;
+       release_sock(sk);
 
-       BT_DBG("%pMR -> %pMR (SID 0x%2.2x)", &iso_pi(sk)->src,
-              &iso_pi(sk)->dst, iso_pi(sk)->bc_sid);
+       BT_DBG("%pMR -> %pMR (SID 0x%2.2x)", &src, &dst, bc_sid);
 
        write_lock(&iso_sk_list.lock);
 
-       if (__iso_get_sock_listen_by_sid(&iso_pi(sk)->src, &iso_pi(sk)->dst,
-                                        iso_pi(sk)->bc_sid))
+       if (__iso_get_sock_listen_by_sid(&src, &dst, bc_sid))
                err = -EADDRINUSE;
 
        write_unlock(&iso_sk_list.lock);
@@ -1231,8 +1253,7 @@ static int iso_listen_bis(struct sock *sk)
        if (err)
                return err;
 
-       hdev = hci_get_route(&iso_pi(sk)->dst, &iso_pi(sk)->src,
-                            iso_pi(sk)->src_type);
+       hdev = hci_get_route(&dst, &src, src_type);
        if (!hdev)
                return -EHOSTUNREACH;
 
@@ -1568,9 +1589,16 @@ static void iso_conn_big_sync(struct sock *sk)
 {
        int err;
        struct hci_dev *hdev;
+       bdaddr_t src, dst;
+       u8 src_type;
+
+       lock_sock(sk);
+       bacpy(&src, &iso_pi(sk)->src);
+       bacpy(&dst, &iso_pi(sk)->dst);
+       src_type = iso_pi(sk)->src_type;
+       release_sock(sk);
 
-       hdev = hci_get_route(&iso_pi(sk)->dst, &iso_pi(sk)->src,
-                            iso_pi(sk)->src_type);
+       hdev = hci_get_route(&dst, &src, src_type);
 
        if (!hdev)
                return;