public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gatt-client: avoid UAF after acquire notify failure
@ 2025-06-25 14:05 Andreas Kemnade
  2025-06-25 15:39 ` bluez.test.bot
  2025-06-26 15:19 ` [PATCH] " Andreas Kemnade
  0 siblings, 2 replies; 3+ messages in thread
From: Andreas Kemnade @ 2025-06-25 14:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andreas Kemnade

If a disconnect happens during AcquireNotify dbus calls, a lot of
debris is left over. Subsequent calls to AcquireNotify will return
NotPermitted and StopNotify leads to an UAF, crashing bluetoothd.

Fix that by also clean up the socket stuff on failure.

This was provoked by putting the device into some RF shielding bag
and open and close it quickly.

Valgrind output:

bluetoothd[26032]: [:1.126:method_call] > org.bluez.GattCharacteristic1.AcquireNotify [#145]
bluetoothd[26032]: [:1.126:error] < org.bluez.Error.NotPermitted [#145]
bluetoothd[26032]: [:1.74:method_call] > org.freedesktop.DBus.Properties.GetAll [#895]
bluetoothd[26032]: [:1.74:method_return] < [#895]
bluetoothd[26032]: [:1.74:method_call] > org.bluez.GattCharacteristic1.StopNotify [#896]
==26032== Invalid read of size 8
==26032==    at 0x1A5721: notify_io_destroy (gatt-client.c:1562)
==26032==    by 0x1A7290: sock_io_destroy (gatt-client.c:1171)
==26032==    by 0x1A7290: destroy_sock (gatt-client.c:1192)
==26032==    by 0x1A73D6: characteristic_stop_notify (gatt-client.c:1698)
==26032==    by 0x1CF478: process_message (object.c:293)
==26032==    by 0x49CB71B: ??? (in /usr/lib/x86_64-linux-gnu/libdbus-1.so.3.32.4)
==26032==    by 0x49BB62A: dbus_connection_dispatch (in /usr/lib/x86_64-linux-gnu/libdbus-1.so.3.32.4)
==26032==    by 0x1CBCEF: message_dispatch (mainloop.c:59)
==26032==    by 0x48BF61E: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.6)
==26032==    by 0x48BF9D7: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.6)
==26032==    by 0x48BFC8E: g_main_loop_run (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.6)
==26032==    by 0x202114: mainloop_run (mainloop-glib.c:65)
==26032==    by 0x202501: mainloop_run_with_signal (mainloop-notify.c:196)
==26032==  Address 0x5a7a430 is 0 bytes inside a block of size 32 free'd
==26032==    at 0x484417B: free (vg_replace_malloc.c:872)
==26032==    by 0x1DEC0F: complete_notify_request (gatt-client.c:1663)
==26032==    by 0x1DEC0F: enable_ccc_callback (gatt-client.c:1735)
==26032==    by 0x1DB8FF: disc_att_send_op (att.c:464)
==26032==    by 0x1DB8FF: disconnect_cb (att.c:676)
==26032==    by 0x2017A4: watch_callback (io-glib.c:173)
==26032==    by 0x48BF61E: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.6)
==26032==    by 0x48BF9D7: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.6)
==26032==    by 0x48BFC8E: g_main_loop_run (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.6)
==26032==    by 0x202114: mainloop_run (mainloop-glib.c:65)
==26032==    by 0x202501: mainloop_run_with_signal (mainloop-notify.c:196)
==26032==    by 0x12E60C: main (main.c:1535)
==26032==  Block was alloc'd at
==26032==    at 0x48417B4: malloc (vg_replace_malloc.c:381)
==26032==    by 0x1D2F4D: util_malloc (util.c:46)
==26032==    by 0x1A5DD3: notify_client_create (gatt-client.c:1426)
==26032==    by 0x1A5EFC: characteristic_acquire_notify (gatt-client.c:1588)
==26032==    by 0x1CF478: process_message (object.c:293)
==26032==    by 0x49CB71B: ??? (in /usr/lib/x86_64-linux-gnu/libdbus-1.so.3.32.4)
==26032==    by 0x49BB62A: dbus_connection_dispatch (in /usr/lib/x86_64-linux-gnu/libdbus-1.so.3.32.4)
==26032==    by 0x1CBCEF: message_dispatch (mainloop.c:59)
==26032==    by 0x48BF61E: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.6)
==26032==    by 0x48BF9D7: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.6)
---
 src/gatt-client.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 3c31a422e..a146d52cb 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1539,8 +1539,7 @@ static void register_notify_io_cb(uint16_t att_ecode, void *user_data)
 	struct bt_gatt_client *gatt = chrc->service->client->gatt;
 
 	if (att_ecode) {
-		queue_remove(chrc->notify_clients, client);
-		notify_client_free(client);
+		destroy_sock(chrc, chrc->notify_io->io);
 		return;
 	}
 
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: gatt-client: avoid UAF after acquire notify failure
  2025-06-25 14:05 [PATCH] gatt-client: avoid UAF after acquire notify failure Andreas Kemnade
@ 2025-06-25 15:39 ` bluez.test.bot
  2025-06-26 15:19 ` [PATCH] " Andreas Kemnade
  1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2025-06-25 15:39 UTC (permalink / raw)
  To: linux-bluetooth, andreas

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

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=975818

---Test result---

Test Summary:
CheckPatch                    PENDING   0.34 seconds
GitLint                       PENDING   0.26 seconds
BuildEll                      PASS      21.42 seconds
BluezMake                     PASS      3050.39 seconds
MakeCheck                     PASS      20.88 seconds
MakeDistcheck                 PASS      214.61 seconds
CheckValgrind                 PASS      292.86 seconds
CheckSmatch                   PASS      315.51 seconds
bluezmakeextell               PASS      129.49 seconds
IncrementalBuild              PENDING   0.25 seconds
ScanBuild                     PASS      926.90 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] gatt-client: avoid UAF after acquire notify failure
  2025-06-25 14:05 [PATCH] gatt-client: avoid UAF after acquire notify failure Andreas Kemnade
  2025-06-25 15:39 ` bluez.test.bot
@ 2025-06-26 15:19 ` Andreas Kemnade
  1 sibling, 0 replies; 3+ messages in thread
From: Andreas Kemnade @ 2025-06-26 15:19 UTC (permalink / raw)
  To: linux-bluetooth

Am Wed, 25 Jun 2025 16:05:45 +0200
schrieb Andreas Kemnade <andreas@kemnade.info>:

> If a disconnect happens during AcquireNotify dbus calls, a lot of
> debris is left over. Subsequent calls to AcquireNotify will return
> NotPermitted and StopNotify leads to an UAF, crashing bluetoothd.
> 
> Fix that by also clean up the socket stuff on failure.
> 
Hmm, I should rather send a dbus answer, too instead of letting the app
waiting for timeout.
I will send an extended version (then with the correct subj prefix)

Regards,
Andreas

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-06-26 15:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-25 14:05 [PATCH] gatt-client: avoid UAF after acquire notify failure Andreas Kemnade
2025-06-25 15:39 ` bluez.test.bot
2025-06-26 15:19 ` [PATCH] " Andreas Kemnade

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox