diff mbox series

[5/7] Bluetooth: introduce hci_conn_free() for better structure

Message ID 20230904221158.35425-6-olivier.lheureux@mind.be (mailing list archive)
State New, archived
Headers show
Series Fix a memory leak in Bluetooth L2CAP | expand

Commit Message

Olivier L'Heureux Sept. 4, 2023, 10:11 p.m. UTC
The bluetooth subsystem uses different sources for different layers or
objects. In particular, the "hci_conn.c" source regroups the handling
of the "struct hci_conn" objects, amongst other things. "hci_conn.c"
contains "hci_conn_add()" to allocate the "struct hci_conn",
"hci_conn_del()" to delete them etc.

One function is lacking: a "hci_conn_free()" to free the "struct
hci_conn". The "kfree()" is in the "bt_link_release()" [1], in
"hci_sysfs.c". "bt_link_release()" is the callback called when
the "struct device" reference count reaches 0. It makes sense that
"bt_link_release()" is in "hci_sysfs.c", with the other functions
related to "struct device" and sysfs, but to respect the structure of
the bluetooth subsystem, "bt_link_release()" should not directly call
"kfree()" on the "struct hci_conn" object. It should call a freeing
function located in "hci_conn.c", so that "hci_conn.c" contains both
the allocation and free of "struct hci_conn" objects.

This improved structure becomes necessary if we want to do more than
just calling "kfree()" in "bt_link_release()". We want to access the
"struct l2cap_conn" associated to the "struct hci_conn", we can do
this in "hci_conn.c", which includes "l2cap.h", while we can't do this
in "hci_sysfs.c", for which "struct l2cap_conn" is opaque.

For those structural reasons:

 1. We create a new "hci_conn_free()" function in "hci_conn.c", whose
    purpose is to free the "struct hci_conn".
 2. We export it by declaring it in
    "include/net/bluetooth/hci_core.h".
 3. Instead of freeing the "struct hci_conn" in "bt_link_release()",
    we call "hci_conn_free()" where we have moved the content of
    "bt_link_release()".

References:
- [1] "bt_link_release"
      <https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/hci_sysfs.c#L13>

Signed-off-by: Olivier L'Heureux <olivier.lheureux@fortrobotics.com>
Signed-off-by: Olivier L'Heureux <olivier.lheureux@mind.be>
---
 include/net/bluetooth/hci_core.h | 1 +
 net/bluetooth/hci_conn.c         | 7 +++++++
 net/bluetooth/hci_sysfs.c        | 4 ++--
 3 files changed, 10 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index d8badb2a28cd..d5a9ef8909d4 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1328,6 +1328,7 @@  int hci_le_create_cis(struct hci_conn *conn);
 
 struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
 			      u8 role);
+void hci_conn_free(struct hci_conn *conn);
 void hci_conn_del(struct hci_conn *conn);
 void hci_conn_hash_flush(struct hci_dev *hdev);
 void hci_conn_check_pending(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 23e635600717..755125403331 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1134,6 +1134,13 @@  static void hci_conn_unlink(struct hci_conn *conn)
 	conn->link = NULL;
 }
 
+void hci_conn_free(struct hci_conn *conn)
+{
+	BT_DBG("kfree(conn %p)", conn);
+
+	kfree(conn);
+}
+
 void hci_conn_del(struct hci_conn *conn)
 {
 	struct hci_dev *hdev = conn->hdev;
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index fc297b651881..b0d841dcf860 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -14,9 +14,9 @@  static void bt_link_release(struct device *dev)
 {
 	struct hci_conn *conn = to_hci_conn(dev);
 
-	BT_DBG("kfree(conn %p)", conn);
+	BT_DBG("dev %p conn %p", dev, conn);
 
-	kfree(conn);
+	hci_conn_free(conn);
 }
 
 static const struct device_type bt_link = {