Bluetooth: ISO: fix refcounting of iso_conn

iso_conn_del() and iso_chan_del() have a race that results to double-put
of iso_conn:

    [Task hdev->workqueue]         [Task 2]
    iso_conn_del                   iso_chan_del
      iso_conn_hold_unless_zero      iso_conn_lock
      iso_conn_lock                  conn->sk = NULL
                                     iso_conn_unlock
      sk = iso_sock_hold(conn)  <---------´
      if (!sk) iso_conn_put          iso_conn_put
      iso_conn_put /* UAF */

The extra put for !sk in iso_conn_del() is currently required since
failing iso_chan_add() may leave iso_conn not associated with any sk.

Fix by having iso_pi(sk)->conn own refcount when non-NULL, so
iso_conn_del does not need to put it.  Adjust the iso_conn_add()
refcounting so that conn is put if it does not get associated with an
sk.

Fixes: dc26097bdb86 ("Bluetooth: ISO: Use kref to track lifetime of iso_conn")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
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 bfd39baa8503d3a1db97d246477dc8974117020a..30de99ba4b30ef20cc6869bb50239b5b5b02e98a 100644 (file)
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -108,9 +108,6 @@ static void iso_conn_free(struct kref *ref)
 
        BT_DBG("conn %p", conn);
 
-       if (conn->sk)
-               iso_pi(conn->sk)->conn = NULL;
-
        if (conn->hcon) {
                conn->hcon->iso_data = NULL;
                if (!test_and_set_bit(ISO_CONN_DROPPED, conn->flags))
@@ -145,6 +142,14 @@ static struct iso_conn *iso_conn_hold_unless_zero(struct iso_conn *conn)
        return conn;
 }
 
+static struct iso_conn *iso_conn_hold(struct iso_conn *conn)
+{
+       BT_DBG("conn %p refcnt %u", conn, kref_read(&conn->ref));
+
+       kref_get(&conn->ref);
+       return conn;
+}
+
 static struct sock *iso_sock_hold(struct iso_conn *conn)
 {
        if (!conn || !bt_sock_linked(&iso_sk_list, conn->sk))
@@ -210,7 +215,6 @@ static struct iso_conn *iso_conn_add(struct hci_conn *hcon)
                        conn->hcon = hcon;
                        iso_conn_unlock(conn);
                }
-               iso_conn_put(conn);
                return conn;
        }
 
@@ -279,10 +283,8 @@ static void iso_conn_del(struct hci_conn *hcon, int err)
        sk = iso_sock_hold(conn);
        iso_conn_unlock(conn);
 
-       if (!sk) {
-               iso_conn_put(conn);
+       if (!sk)
                goto done;
-       }
 
        iso_sock_disable_timer(sk);
 
@@ -320,7 +322,7 @@ static int __iso_chan_add(struct iso_conn *conn, struct sock *sk,
                return -EIO;
        }
 
-       iso_pi(sk)->conn = conn;
+       iso_pi(sk)->conn = iso_conn_hold(conn);
        conn->sk = sk;
        clear_bit(ISO_CONN_DROPPED, conn->flags);
 
@@ -427,6 +429,7 @@ static int iso_connect_bis(struct sock *sk)
        }
 
        err = iso_chan_add(conn, sk, NULL);
+       iso_conn_put(conn);
        if (err)
                goto unlock;
 
@@ -529,6 +532,7 @@ static int iso_connect_cis(struct sock *sk)
        }
 
        err = iso_chan_add(conn, sk, NULL);
+       iso_conn_put(conn);
        if (err)
                goto unlock;
 
@@ -1310,10 +1314,9 @@ static int iso_listen_bis(struct sock *sk)
        }
 
        err = iso_chan_add(conn, sk, NULL);
-       if (err) {
-               hci_conn_drop(hcon);
+       iso_conn_put(conn);
+       if (err)
                goto unlock;
-       }
 
 unlock:
        release_sock(sk);
@@ -2551,8 +2554,10 @@ static void iso_connect_cfm(struct hci_conn *hcon, __u8 status)
                struct iso_conn *conn;
 
                conn = iso_conn_add(hcon);
-               if (conn)
+               if (conn) {
                        iso_conn_ready(conn);
+                       iso_conn_put(conn);
+               }
        } else {
                iso_conn_del(hcon, bt_to_errno(status));
        }