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 | 7 +++++++
net/bluetooth/l2cap_core.c | 29 ++++++++++++++++++++++++++---
3 files changed, 35 insertions(+), 3 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..814d8f3b029e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -2737,6 +2737,9 @@ void hci_chan_del(struct hci_chan *chan)
struct hci_conn *conn = chan->conn;
struct hci_dev *hdev = conn->hdev;
+ if (!conn)
+ return;
+
BT_DBG("%s hcon %p chan %p", hdev->name, conn, chan);
list_del_rcu(&chan->list);
@@ -2746,6 +2749,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
Hi Luiz,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-L2CAP-Fix-leaking-l2cap_conn-objects/20230914-034053
base: linus/master
patch link: https://lore.kernel.org/r/20230913193839.3029428-1-luiz.dentz%40gmail.com
patch subject: [PATCH] Bluetooth: L2CAP: Fix leaking l2cap_conn objects
config: x86_64-randconfig-161-20230914 (https://download.01.org/0day-ci/archive/20230914/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230914/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/
smatch warnings:
net/bluetooth/hci_conn.c:2725 hci_chan_del() warn: variable dereferenced before check 'conn' (see line 2723)
vim +/conn +2725 net/bluetooth/hci_conn.c
9472007c62ecc8 Andrei Emeltchenko 2012-09-06 2720 void hci_chan_del(struct hci_chan *chan)
73d80deb7bdf01 Luiz Augusto von Dentz 2011-11-02 2721 {
73d80deb7bdf01 Luiz Augusto von Dentz 2011-11-02 2722 struct hci_conn *conn = chan->conn;
73d80deb7bdf01 Luiz Augusto von Dentz 2011-11-02 @2723 struct hci_dev *hdev = conn->hdev;
^^^^^^^^^^
Dereference
73d80deb7bdf01 Luiz Augusto von Dentz 2011-11-02 2724
6fc2f406e4158e Luiz Augusto von Dentz 2023-09-13 @2725 if (!conn)
^^^^^
Too late
6fc2f406e4158e Luiz Augusto von Dentz 2023-09-13 2726 return;
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki