All of lore.kernel.org
 help / color / mirror / Atom feed
From: Olivier L'Heureux <olivier.lheureux@mind.be>
To: Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: linux-bluetooth@vger.kernel.org,
	Olivier L'Heureux <olivier.lheureux@mind.be>
Subject: [PATCH 5/7] Bluetooth: introduce hci_conn_free() for better structure
Date: Tue,  5 Sep 2023 00:11:56 +0200	[thread overview]
Message-ID: <20230904221158.35425-6-olivier.lheureux@mind.be> (raw)
In-Reply-To: <20230904221158.35425-1-olivier.lheureux@mind.be>

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 --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 = {
-- 
2.39.2


  parent reply	other threads:[~2023-09-04 22:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-04 22:11 [PATCH RFC 0/7] Fix a memory leak in Bluetooth L2CAP Olivier L'Heureux
2023-09-04 22:11 ` [PATCH 1/7] ARM: dts: stm32: Add Bluetooth (usart2) feature on stm32mp157x Olivier L'Heureux
2023-09-04 22:38   ` Fix a memory leak in Bluetooth L2CAP bluez.test.bot
2023-09-04 22:11 ` [PATCH 2/7] Bluetooth: add many traces for allocation/free/refcounting Olivier L'Heureux
2023-09-04 22:11 ` [PATCH 3/7] Bluetooth: L2CAP: use refcount on struct l2cap_chan->conn Olivier L'Heureux
2023-09-04 22:11 ` [PATCH 4/7] Bluetooth: L2CAP: free the leaking struct l2cap_conn Olivier L'Heureux
2023-09-04 22:11 ` Olivier L'Heureux [this message]
2023-09-04 22:11 ` [PATCH 6/7] Bluetooth: L2CAP: inc refcount if reuse " Olivier L'Heureux
2023-09-04 22:11 ` [PATCH 7/7] Bluetooth: unlink objects to avoid use-after-free Olivier L'Heureux
2023-09-05 20:42 ` [PATCH RFC 0/7] Fix a memory leak in Bluetooth L2CAP Luiz Augusto von Dentz
2023-09-12 19:19   ` Luiz Augusto von Dentz
2023-09-13 22:25   ` Olivier L'Heureux
2023-09-14 21:17     ` Luiz Augusto von Dentz
2023-11-07  8:46       ` Marleen Vos
2023-11-07 15:10         ` Luiz Augusto von Dentz
     [not found]           ` <CAJav_R4Dp1rddk67vNDPvwS4208pk7-aORXM0KiwXPaOkXmGkw@mail.gmail.com>
2023-11-13 14:58             ` Luiz Augusto von Dentz
2023-11-13 15:01               ` Marleen Vos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230904221158.35425-6-olivier.lheureux@mind.be \
    --to=olivier.lheureux@mind.be \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.