[RFC BlueZ] media: Fix possible crash on exit/adapter removal

[RFC BlueZ] media: Fix possible crash on exit/adapter removal

[Bastien Nocera]

Nothing protects media_endpoint_remove() from being called multiple
times for the same structure. Before a g_free() call is made on
endpoint->capabilities, there are NULL checks, and NULL setting,
for every variable that might get modified, so a second call to the same
function, even though it's still using-after-free, is only
reading-after-free, and might crash at the first attempt at modifying
that freed memory.

The reason why this function might be called multiple times is because
in some circumstances, another signal might be received that the
endpoint is getting removed while we're already in the process of
removing that endpoint.

For example, release_endpoint() (which should appear in between
path_free() and media_endpoint_remove() in the below backtrace, as
that's the function called at profiles/audio/media.c:3651), will send a
D-Bus message which it then waits for the answer to, meaning that other
D-Bus message could be received while we're waiting for the answer, and
then destroying the endpoint.

 #11 media_endpoint_destroy at profiles/audio/media.c:231
 #12 media_endpoint_remove at profiles/audio/media.c:314
 #13 path_free at profiles/audio/media.c:3651
 #14 remove_interface at gdbus/object.c:742
 #15 g_dbus_unregister_interface at gdbus/object.c:1499
 #16 g_slist_foreach at ../glib/gslist.c:837
 #17 unload_drivers at src/adapter.c:5932
 #18 adapter_remove at src/adapter.c:7088
 #19 adapter_unregister at src/adapter.c:9504
 #20 index_removed at src/adapter.c:10693
 #21 queue_foreach at src/shared/queue.c:207
 #23 process_notify at src/shared/mgmt.c:349
 #24 can_read_data at src/shared/mgmt.c:409
 #25 watch_callback at src/shared/io-glib.c:173
 #27 g_main_context_dispatch_unlocked at ../glib/gmain.c:4451
 #28 g_main_context_iterate_unlocked at ../glib/gmain.c:4516
 #30 mainloop_run at src/shared/mainloop-glib.c:65
 #31 mainloop_run_with_signal at src/shared/mainloop-notify.c:196

in profiles/audio/media.c:
 231         g_free(endpoint->capabilities);

See https://bugzilla.redhat.com/show_bug.cgi?id=2467980
---
 profiles/audio/media.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index cdaafb04e38c..ad31872c6431 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -611,6 +611,7 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 static void release_endpoint(struct media_endpoint *endpoint)
 {
 	DBusMessage *msg;
+	struct media_adapter *adapter = endpoint->adapter;
 
 	DBG("sender=%s path=%s", endpoint->sender, endpoint->path);
 
@@ -631,7 +632,9 @@ static void release_endpoint(struct media_endpoint *endpoint)
 	g_dbus_send_message(btd_get_dbus_connection(), msg);
 
 done:
-	media_endpoint_remove(endpoint);
+	/* Make sure endpoint didn't already get removed */
+	if (g_slist_find(adapter->endpoints, endpoint))
+		media_endpoint_remove(endpoint);
 }
 
 static const char *get_name(struct a2dp_sep *sep, void *user_data)
-- 
2.54.0

RE: [RFC,BlueZ] media: Fix possible crash on exit/adapter removal

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 989 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=1093445

---Test result---

Test Summary:
CheckPatch                    PASS      0.43 seconds
GitLint                       PASS      0.29 seconds
BuildEll                      PASS      20.29 seconds
BluezMake                     PASS      657.53 seconds
MakeCheck                     PASS      2.98 seconds
MakeDistcheck                 PASS      246.45 seconds
CheckValgrind                 PASS      226.08 seconds
CheckSmatch                   PASS      350.07 seconds
bluezmakeextell               PASS      181.48 seconds
IncrementalBuild              PASS      664.24 seconds
ScanBuild                     PASS      1027.26 seconds



https://github.com/bluez/bluez/pull/2122

---
Regards,
Linux Bluetooth