2023-09-16 14:07:18

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH v3] Bluetooth: L2CAP: Fix leaking l2cap_conn objects

From: Luiz Augusto von Dentz <[email protected]>

l2cap_conn objects must be cleanup whenever its hci_chan is deleted by
hci_chan_list_flush since it means the connection attempt is being
aborted prematurely thus no calls to connect_cfm/disconnect_cfm would
be generated causing the l2cap_conn object to leak.

Fixes: 73d80deb7bdf ("Bluetooth: prioritizing data over HCI")
Reported-by: Olivier L'Heureux <[email protected]>
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_conn.c | 10 ++++++++--
net/bluetooth/l2cap_core.c | 29 ++++++++++++++++++++++++++---
3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index bbad301f5781..21459c38a074 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -795,6 +795,8 @@ struct hci_chan {
unsigned int sent;
__u8 state;
bool amp;
+
+ void (*cleanup)(struct hci_chan *chan);
};

struct hci_conn_params {
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e62a5f368a51..c4e16e92446c 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -2735,9 +2735,11 @@ struct hci_chan *hci_chan_create(struct hci_conn *conn)
void hci_chan_del(struct hci_chan *chan)
{
struct hci_conn *conn = chan->conn;
- struct hci_dev *hdev = conn->hdev;

- BT_DBG("%s hcon %p chan %p", hdev->name, conn, chan);
+ if (!conn)
+ return;
+
+ bt_dev_dbg(conn->hdev, "hcon %p chan %p", conn, chan);

list_del_rcu(&chan->list);

@@ -2746,6 +2748,10 @@ void hci_chan_del(struct hci_chan *chan)
/* Prevent new hci_chan's to be created for this hci_conn */
set_bit(HCI_CONN_DROP, &conn->flags);

+ if (chan->cleanup)
+ chan->cleanup(chan);
+
+ chan->conn = NULL;
hci_conn_put(conn);

skb_queue_purge(&chan->data_q);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 17ca13e8c044..a791f28ccd6a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1896,6 +1896,8 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)

BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);

+ hcon->l2cap_data = NULL;
+
kfree_skb(conn->rx_skb);

skb_queue_purge(&conn->pending_rx);
@@ -1931,13 +1933,15 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)

mutex_unlock(&conn->chan_lock);

- hci_chan_del(conn->hchan);
+ if (conn->hchan) {
+ conn->hchan->cleanup = NULL;
+ hci_chan_del(conn->hchan);
+ conn->hchan = NULL;
+ }

if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
cancel_delayed_work_sync(&conn->info_timer);

- hcon->l2cap_data = NULL;
- conn->hchan = NULL;
l2cap_conn_put(conn);
}

@@ -7830,6 +7834,24 @@ static void process_pending_rx(struct work_struct *work)
l2cap_recv_frame(conn, skb);
}

+static void l2cap_conn_hchan_cleanup(struct hci_chan *hchan)
+{
+ struct hci_conn *hcon = hchan->conn;
+ struct l2cap_conn *conn;
+
+ if (!hcon)
+ return;
+
+ conn = hcon->l2cap_data;
+ if (!conn)
+ return;
+
+ /* hci_chan_del has been called so we shouldn't call it gain. */
+ conn->hchan = NULL;
+
+ l2cap_conn_del(hcon, bt_to_errno(HCI_ERROR_LOCAL_HOST_TERM));
+}
+
static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
{
struct l2cap_conn *conn = hcon->l2cap_data;
@@ -7852,6 +7874,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
hcon->l2cap_data = conn;
conn->hcon = hci_conn_get(hcon);
conn->hchan = hchan;
+ hchan->cleanup = l2cap_conn_hchan_cleanup;

BT_DBG("hcon %p conn %p hchan %p", hcon, conn, hchan);

--
2.41.0