Bluetooth: Fix hci_conn reference counting for auto-connections

Recently the LE passive scanning and auto-connections feature was
introduced. It uses the hci_connect_le() API which returns a hci_conn
along with a reference count to that object. All previous users would
tie this returned reference to some existing object, such as an L2CAP
channel, and there'd be no leaked references this way. For
auto-connections however the reference was returned but not stored
anywhere, leaving established connections with one higher reference
count than they should have.

Instead of playing special tricks with hci_conn_hold/drop this patch
associates the returned reference from hci_connect_le() with the object
that in practice does own this reference, i.e. the hci_conn_params
struct that caused us to initiate a connection in the first place. Once
the connection is established or fails to establish this reference is
removed appropriately.

One extra thing needed is to call hci_pend_le_actions_clear() before
calling hci_conn_hash_flush() so that the reference is cleared before
the hci_conn objects are fully removed.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b5d5af3aa469961af36b30b0aea2da1d4e959bac..6f884e6c731e41daccdf4119cb1f0f8a4ee9335f 100644 (file)
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -464,6 +464,8 @@ struct hci_conn_params {
                HCI_AUTO_CONN_ALWAYS,
                HCI_AUTO_CONN_LINK_LOSS,
        } auto_connect;
+
+       struct hci_conn *conn;
 };
 
 extern struct list_head hci_dev_list;

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b50dabb3f86ab49667cb939af29cef53a03b2ac2..faff6247ac8fb8769bf58f235fd92c0deab6aaf1 100644 (file)
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -589,6 +589,14 @@ EXPORT_SYMBOL(hci_get_route);
 void hci_le_conn_failed(struct hci_conn *conn, u8 status)
 {
        struct hci_dev *hdev = conn->hdev;
+       struct hci_conn_params *params;
+
+       params = hci_pend_le_action_lookup(&hdev->pend_le_conns, &conn->dst,
+                                          conn->dst_type);
+       if (params && params->conn) {
+               hci_conn_drop(params->conn);
+               params->conn = NULL;
+       }
 
        conn->state = BT_CLOSED;
 

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index c32d361c0cf766478f80fb9ed8a4884f53451122..1d9c29a00568d9b6ac3eea93848ec6637e5a5a5d 100644 (file)
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2536,8 +2536,13 @@ static void hci_pend_le_actions_clear(struct hci_dev *hdev)
 {
        struct hci_conn_params *p;
 
-       list_for_each_entry(p, &hdev->le_conn_params, list)
+       list_for_each_entry(p, &hdev->le_conn_params, list) {
+               if (p->conn) {
+                       hci_conn_drop(p->conn);
+                       p->conn = NULL;
+               }
                list_del_init(&p->action);
+       }
 
        BT_DBG("All LE pending actions cleared");
 }
@@ -2578,8 +2583,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 
        hci_dev_lock(hdev);
        hci_inquiry_cache_flush(hdev);
-       hci_conn_hash_flush(hdev);
        hci_pend_le_actions_clear(hdev);
+       hci_conn_hash_flush(hdev);
        hci_dev_unlock(hdev);
 
        hci_notify(hdev, HCI_DEV_DOWN);
@@ -3727,6 +3732,9 @@ void hci_conn_params_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
        if (!params)
                return;
 
+       if (params->conn)
+               hci_conn_drop(params->conn);
+
        list_del(&params->action);
        list_del(&params->list);
        kfree(params);
@@ -3757,6 +3765,8 @@ void hci_conn_params_clear_all(struct hci_dev *hdev)
        struct hci_conn_params *params, *tmp;
 
        list_for_each_entry_safe(params, tmp, &hdev->le_conn_params, list) {
+               if (params->conn)
+                       hci_conn_drop(params->conn);
                list_del(&params->action);
                list_del(&params->list);
                kfree(params);

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index be35598984d9b9b3120cd178f67f0418858b2774..a6000823f0ff34d938631bed2e39a6b17a26ac13 100644 (file)
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4221,8 +4221,13 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
        hci_proto_connect_cfm(conn, ev->status);
 
        params = hci_conn_params_lookup(hdev, &conn->dst, conn->dst_type);
-       if (params)
+       if (params) {
                list_del_init(&params->action);
+               if (params->conn) {
+                       hci_conn_drop(params->conn);
+                       params->conn = NULL;
+               }
+       }
 
 unlock:
        hci_update_background_scan(hdev);
@@ -4304,8 +4309,16 @@ static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
 
        conn = hci_connect_le(hdev, addr, addr_type, BT_SECURITY_LOW,
                              HCI_LE_AUTOCONN_TIMEOUT, HCI_ROLE_MASTER);
-       if (!IS_ERR(conn))
+       if (!IS_ERR(conn)) {
+               /* Store the pointer since we don't really have any
+                * other owner of the object besides the params that
+                * triggered it. This way we can abort the connection if
+                * the parameters get removed and keep the reference
+                * count consistent once the connection is established.
+                */
+               params->conn = conn;
                return;
+       }
 
        switch (PTR_ERR(conn)) {
        case -EBUSY: