We have detected a memory leak of "struct l2cap_conn" objects, and of
"struct hci_conn" in the bluetooth subsystem [12]. The second is a
consequence of the first. This memory leak was also detected by syzbot
[13].
"struct l2cap_conn" is allocated in "l2cap_conn_add()" [2]. "struct
l2cap_conn" is reference-counted [1] by its "ref" member [3]: every
holder of a reference to it should increment the reference counter by
calling "l2cap_conn_get()" [4], and every reference holder that stops
keeping the reference should call "l2cap_conn_put()" [5]. The "struct
l2cap_conn" is freed when the counter reaches 0, meaning there is no
reference holder any more.
This mechanism is already used by the "hidp_session_new()" [6] and
"session_free()" [7] functions that create and delete the "struct
hidp_session" [8] objects, which contains a "conn" reference to a
"struct l2cap_conn".
The same reference counting mechanism is not used for the "conn"
reference kept in the "struct l2cap_chan" [9] object. There were two
places where the reference counting was not applied:
1. In "__l2cap_chan_add()" [11], the "struct l2cap_conn" reference is
stored in the "conn" member of the "struct l2cap_chan" [9].
2. In "l2cap_chan_del()" [10], the "conn" member of the "struct
l2cap_chan" is set to NULL.
To apply the reference counting to the "conn" member of the "struct
l2cap_chan", we use "l2cap_conn_get()" in "__l2cap_chan_add()", where
we store the reference, and "l2cap_conn_put()" in "l2cap_chan_del()",
where we drop the reference.
Handling the "conn" member of the "struct l2cap_chan" with the
existing reference counter will help to fix the kernel memory leak in
a following commit.
References:
- [1] "Adding reference counters (krefs) to kernel objects"
<https://www.kernel.org/doc/html/v6.5/core-api/kref.html>
- [2] "l2cap_conn_add()"
<https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/l2cap_core.c#L7833>
- [3] "struct l2cap_conn"
<https://elixir.bootlin.com/linux/v6.5/source/include/net/bluetooth/l2cap.h#L674>
- [4] "l2cap_conn_get()"
<https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/l2cap_core.c#L1952>
- [5] "l2cap_conn_put()"
<https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/l2cap_core.c#L1959>
- [6] "hidp_session_new()"
<https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/hidp/core.c#L910>
- [7] "session_free()"
<https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/hidp/core.c#L979>
- [8] "struct hidp_session"
<https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/hidp/hidp.h#L137>
- [9] "struct l2cap_chan"
<https://elixir.bootlin.com/linux/v6.5/source/include/net/bluetooth/l2cap.h#L540>
- [10] "l2cap_chan_del()"
<https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/l2cap_core.c#L642>
- [11] "__l2cap_chan_add()"
<https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/l2cap_core.c#L583>
- [12] "ble-memleak-repro"
<https://gitlab.com/essensium-mind/ble-memleak-repro.git>
- [13] "[syzbot] [bluetooth?] memory leak in hci_conn_add (2)"
<https://lore.kernel.org/linux-bluetooth/0000000000000fd01206046e7410@xxxxxxxxxx/T/#u>
Signed-off-by: Olivier L'Heureux <olivier.lheureux@xxxxxxxxxxxxxxxx>
Signed-off-by: Olivier L'Heureux <olivier.lheureux@xxxxxxx>
---
net/bluetooth/l2cap_core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index c749b434df97..768632fba6f8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -587,7 +587,7 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
conn->disc_reason = HCI_ERROR_REMOTE_USER_TERM;
- chan->conn = conn;
+ chan->conn = l2cap_conn_get(conn);
switch (chan->chan_type) {
case L2CAP_CHAN_CONN_ORIENTED:
@@ -669,6 +669,8 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
if (mgr && mgr->bredr_chan == chan)
mgr->bredr_chan = NULL;
+
+ l2cap_conn_put(conn);
}
if (chan->hs_hchan) {
--
2.39.2
