gatt-client: fix use-after-free

PATCH Applied Lasse Dalegaard 4 participants 10 messages Jan 8 – Jan 9 Bluetooth drivers

Share & tools

Lasse Dalegaard

author

2026-01-08 10:01 UTC #0 lore raw

PATCH 1/2 gatt-client: fix use-after-free

Clients with an acquired characteristic subscription would free the client during disconnect. When notify_io_destroy then tries to remove the client, it'll use-after-free.

Add another reference when allocating the notify_io and unref it again in notify_io_destroy.

---
 src/gatt-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

src/gatt-client.c +2 -1

index 44ec95db0..374e67c34 100644
@@ -1566,6 +1566,7 @@ static void notify_io_destroy(void *data)
 
 	if (queue_remove(client->chrc->notify_clients, client))
 		notify_client_unref(client);
+	notify_client_unref(client);
 }
 
 static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,

@@ -1607,7 +1608,7 @@ static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,
 	queue_push_tail(chrc->notify_clients, client);
 
 	chrc->notify_io = new0(struct sock_io, 1);
-	chrc->notify_io->data = client;
+	chrc->notify_io->data = notify_client_ref(client);
 	chrc->notify_io->msg = dbus_message_ref(msg);
 	chrc->notify_io->destroy = notify_io_destroy;

Signature · 1 line

2.52.0

Lasse Dalegaard

author

2026-01-08 10:01 UTC #1 lore raw

PATCH 2/2 device: fix memory leak

device_add_eir_uuids creates a list of added UUIDs, but it was never freed.

---
 src/device.c | 1 +
 1 file changed, 1 insertion(+)

src/device.c +1 -0

index c8aaf042f..0842becde 100644
@@ -2453,6 +2453,7 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
 	}
 
 	device_probe_profiles(dev, added);
+	g_slist_free(added);
 }
 
 static void add_manufacturer_data(void *data, void *user_data)

Signature · 1 line

2.52.0

Luiz Augusto von Dentz

maintainer

2026-01-08 15:28 UTC #2 lore raw

Re: [PATCH BlueZ 2/2] device: fix memory leak

Hi Lasse,

Quoted 3 lines · Lasse Dalegaard

device_add_eir_uuids creates a list of added UUIDs, but it was never freed.

This seems valid, how did you find it though, I don't recall seeing it with the likes of valgrind, anyway it would be great to have the information how this was detected.

Quoted 20 lines

---
 src/device.c | 1 +
 1 file changed, 1 insertion(+)

src/device.c +1 -1

index c8aaf042f..0842becde 100644
@@ -2453,6 +2453,7 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
        }
 
        device_probe_profiles(dev, added);
+       g_slist_free(added);
 }
 
 static void add_manufacturer_data(void *data, void *user_data)
--

2.52.0

Signature · 1 line

Luiz Augusto von Dentz

Lasse Dalegaard

author

2026-01-08 18:50 UTC #3 lore raw

Re: [PATCH BlueZ 2/2] device: fix memory leak

Hi Luiz,

Quoted 11 lines · Luiz Augusto von Dentz

Hi Lasse,
Quoted 3 lines · Lasse Dalegaard
device_add_eir_uuids creates a list of added UUIDs, but it was never freed.
This seems valid, how did you find it though, I don't recall seeing it with the likes of valgrind, anyway it would be great to have the information how this was detected.

This was detected with the address sanitizer leak checker.

Quoted 16 lines

---
 src/device.c | 1 +
 1 file changed, 1 insertion(+)

src/device.c +1 -0

index c8aaf042f..0842becde 100644
@@ -2453,6 +2453,7 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
        }
 
        device_probe_profiles(dev, added);
+       g_slist_free(added);
 }

Quoted 9 lines

Quoted 5 lines
src/device.c +0 -1
 static void add_manufacturer_data(void *data, void *user_data)
--
2.52.0
--
Luiz Augusto von Dentz

Best regards,
Lasse

Luiz Augusto von Dentz

maintainer

2026-01-08 19:08 UTC #4 lore raw

Re: [PATCH BlueZ 2/2] device: fix memory leak

Hi Lasse,

Quoted 10 lines · Lasse Dalegaard

Hi Luiz,
Quoted 5 lines · Luiz Augusto von Dentz
Hi Lasse,
Quoted 1 line · Lasse Dalegaard

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
device_add_eir_uuids creates a list of added UUIDs, but it was never freed.
This seems valid, how did you find it though, I don't recall seeing it with the likes of valgrind, anyway it would be great to have the information how this was detected.
This was detected with the address sanitizer leak checker.

Interesting, Ive haven't seen it although I have the sanitizer enabled, I think, but anyway it seem to be correct, do add its output though so in case someone e.g google for it it shows up in the git commit message.

Quoted 28 lines

Quoted 16 lines

---
 src/device.c | 1 +
 1 file changed, 1 insertion(+)

src/device.c +1 -0

index c8aaf042f..0842becde 100644
@@ -2453,6 +2453,7 @@ void device_add_eir_uuids(struct btd_device *dev, GSList *uuids)
        }
 
        device_probe_profiles(dev, added);
+       g_slist_free(added);
 }

Quoted 9 lines

Quoted 5 lines
src/device.c +0 -1
 static void add_manufacturer_data(void *data, void *user_data)
--
2.52.0
--
Luiz Augusto von Dentz

Best regards,
Lasse

Signature · 1 line

Luiz Augusto von Dentz

[email protected]

2026-01-08 11:10 UTC #5 lore raw

In reply to Lasse Dalegaard

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1039830

---Test result---

Test Summary:

CheckPatch                    PENDING   0.38 seconds
GitLint                       PENDING   0.33 seconds
BuildEll                      PASS      17.55 seconds
BluezMake                     PASS      600.91 seconds
MakeCheck                     PASS      21.64 seconds
MakeDistcheck                 PASS      218.34 seconds
CheckValgrind                 PASS      287.53 seconds
CheckSmatch                   PASS      313.91 seconds
bluezmakeextell               PASS      167.39 seconds
IncrementalBuild              PENDING   0.40 seconds
ScanBuild                     PASS      921.61 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series Output:

---
Regards,
Linux Bluetooth

Luiz Augusto von Dentz

maintainer

2026-01-08 15:23 UTC #6 lore raw

In reply to Lasse Dalegaard

Hi Lasse,

Quoted 7 lines · Lasse Dalegaard

Clients with an acquired characteristic subscription would free the client during disconnect. When notify_io_destroy then tries to remove the client, it'll use-after-free.
Add another reference when allocating the notify_io and unref it again in notify_io_destroy.

Please add a backtrace to the patch description.

Quoted 29 lines

---
 src/gatt-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

src/gatt-client.c +2 -2

index 44ec95db0..374e67c34 100644
@@ -1566,6 +1566,7 @@ static void notify_io_destroy(void *data)
 
        if (queue_remove(client->chrc->notify_clients, client))
                notify_client_unref(client);
+       notify_client_unref(client);
 }
 
 static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,

@@ -1607,7 +1608,7 @@ static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,
        queue_push_tail(chrc->notify_clients, client);
 
        chrc->notify_io = new0(struct sock_io, 1);
-       chrc->notify_io->data = client;
+       chrc->notify_io->data = notify_client_ref(client);
        chrc->notify_io->msg = dbus_message_ref(msg);
        chrc->notify_io->destroy = notify_io_destroy;
 
--

2.52.0

Signature · 1 line

Luiz Augusto von Dentz

Lasse Dalegaard

author

2026-01-08 18:46 UTC #7 lore raw

Hi Luiz,

Quoted 5 lines · Luiz Augusto von Dentz

Hi Lasse,
Quoted 1 line · Lasse Dalegaard

Quoted 8 lines

Quoted 6 lines
Clients with an acquired characteristic subscription would free the client during disconnect. When notify_io_destroy then tries to remove the client, it'll use-after-free.
Add another reference when allocating the notify_io and unref it again in notify_io_destroy.
Please add a backtrace to the patch description.

Are you looking for backtraces on how this was discovered or something else? There could be multiple paths that cause the use-after-free.

Quoted 26 lines

---
 src/gatt-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

src/gatt-client.c +2 -1

index 44ec95db0..374e67c34 100644
@@ -1566,6 +1566,7 @@ static void notify_io_destroy(void *data)
 
        if (queue_remove(client->chrc->notify_clients, client))
                notify_client_unref(client);
+       notify_client_unref(client);
 }
 
 static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,

@@ -1607,7 +1608,7 @@ static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,
        queue_push_tail(chrc->notify_clients, client);
 
        chrc->notify_io = new0(struct sock_io, 1);
-       chrc->notify_io->data = client;
+       chrc->notify_io->data = notify_client_ref(client);
        chrc->notify_io->msg = dbus_message_ref(msg);
        chrc->notify_io->destroy = notify_io_destroy;

Quoted 8 lines

Quoted 4 lines
--
2.52.0
--
Luiz Augusto von Dentz

Best regards,
Lasse

Luiz Augusto von Dentz

maintainer

2026-01-08 19:06 UTC #8 lore raw

Hi Lasse,

Quoted 14 lines · Lasse Dalegaard

Hi Luiz,
Quoted 9 lines · Luiz Augusto von Dentz
Hi Lasse,
Quoted 5 lines · Lasse Dalegaard
Clients with an acquired characteristic subscription would free the client during disconnect. When notify_io_destroy then tries to remove the client, it'll use-after-free.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
Add another reference when allocating the notify_io and unref it again in notify_io_destroy.
Please add a backtrace to the patch description.
Are you looking for backtraces on how this was discovered or something else? There could be multiple paths that cause the use-after-free.

How you discovered it

Quoted 37 lines

Quoted 26 lines

---
 src/gatt-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

src/gatt-client.c +2 -1

index 44ec95db0..374e67c34 100644
@@ -1566,6 +1566,7 @@ static void notify_io_destroy(void *data)
 
        if (queue_remove(client->chrc->notify_clients, client))
                notify_client_unref(client);
+       notify_client_unref(client);
 }
 
 static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,

@@ -1607,7 +1608,7 @@ static DBusMessage *characteristic_acquire_notify(DBusConnection *conn,
        queue_push_tail(chrc->notify_clients, client);
 
        chrc->notify_io = new0(struct sock_io, 1);
-       chrc->notify_io->data = client;
+       chrc->notify_io->data = notify_client_ref(client);
        chrc->notify_io->msg = dbus_message_ref(msg);
        chrc->notify_io->destroy = notify_io_destroy;

Quoted 8 lines

Quoted 4 lines
--
2.52.0
--
Luiz Augusto von Dentz

Best regards,
Lasse

Signature · 1 line

Luiz Augusto von Dentz

[email protected]

CI Applied

2026-01-09 22:43 UTC #9 lore raw

In reply to Lasse Dalegaard

This series was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <[email protected]>:

Hello:

This series was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <[email protected]>:

Quoted 8 lines · On Thu, 8 Jan 2026 11:01:35 +0100 you wrote:

Clients with an acquired characteristic subscription would free the client during disconnect. When notify_io_destroy then tries to remove the client, it'll use-after-free.
Add another reference when allocating the notify_io and unref it again in notify_io_destroy.
[...]

Here is the summary with links:
- [BlueZ,1/2] gatt-client: fix use-after-free

    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a94f994201a6
  - [BlueZ,2/2] device: fix memory leak
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=024b148d73ae

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html