| Message ID | 20231004075646.4277-2-silviu.barbulescu@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Update transport acquire/release flow BAP bcast source | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
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=789831 ---Test result--- Test Summary: CheckPatch PASS 1.12 seconds GitLint PASS 0.70 seconds BuildEll PASS 27.88 seconds BluezMake PASS 774.89 seconds MakeCheck PASS 12.10 seconds MakeDistcheck PASS 160.36 seconds CheckValgrind PASS 257.00 seconds CheckSmatch PASS 349.02 seconds bluezmakeextell PASS 106.28 seconds IncrementalBuild PASS 1308.43 seconds ScanBuild PASS 1032.29 seconds --- Regards, Linux Bluetooth
Hi Silviu,
this patch introduced a use-after-free, reproducer:
- connect to a2dp sink
- start playing
- pause and wait until stream suspends
- bluetoothd crashes in a2dp_suspend_complete()
Here's the output running with address sanitizer:
bluetoothd[181120]: profiles/audio/a2dp.c:suspend_cfm() Source
0x60600001e500: Suspend_Cfm
=================================================================
==181120==ERROR: AddressSanitizer: heap-use-after-free on address
0x60300005a730 at pc 0xaaaaf9dbeea8 bp 0xfffff4d3b690 sp 0xfffff4d3b6a8
READ of size 8 at 0x60300005a730 thread T0
#0 0xaaaaf9dbeea4 in a2dp_suspend_complete
profiles/audio/transport.c:426
#1 0xaaaaf9d7d37c in finalize_suspend profiles/audio/a2dp.c:376
#2 0xaaaaf9d7e060 in suspend_cfm profiles/audio/a2dp.c:1276
#3 0xaaaaf9da0ddc in avdtp_delay_report_resp
profiles/audio/avdtp.c:2928
#4 0xaaaaf9da0ddc in avdtp_parse_resp profiles/audio/avdtp.c:2997
#5 0xaaaaf9da0ddc in session_cb profiles/audio/avdtp.c:2286
#6 0xffff6e77030c in g_main_dispatch ../glib/gmain.c:3476
#7 0xffff6e77030c in g_main_context_dispatch_unlocked
../glib/gmain.c:4284
#8 0xffff6e7ce598 in g_main_context_iterate_unlocked.isra.0
../glib/gmain.c:4349
#9 0xffff6e771a60 in g_main_loop_run
(/lib64/libglib-2.0.so.0+0x61a60) (BuildId:
7d17ee836a99abf35136ebb46126a95dee66511a)
#10 0xaaaafa0ad0d8 in mainloop_run src/shared/mainloop-glib.c:66
#11 0xaaaafa0ad920 in mainloop_run_with_signal
src/shared/mainloop-notify.c:188
#12 0xaaaaf9d5489c in main src/main.c:1452
#13 0xffff6dd209d8 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
#14 0xffff6dd20aac in __libc_start_main_impl ../csu/libc-start.c:360
#15 0xaaaaf9d5686c in _start
(/home/jonas/git/bluez/src/bluetoothd+0x55686c) (BuildId:
8922b026a55aac729ed13de54b3a622a31b5bb2b)
0x60300005a730 is located 0 bytes inside of 32-byte region
[0x60300005a730,0x60300005a750)
freed by thread T0 here:
#0 0xffff6ea24638 in __interceptor_free.part.0
(/lib64/libasan.so.8+0xc4638) (BuildId:
a77e9fa1e1448d41e9a8ddaf28aa5612f3ffc397)
#1 0xffff6e773114 in g_free (/lib64/libglib-2.0.so.0+0x63114)
(BuildId: 7d17ee836a99abf35136ebb46126a95dee66511a)
#2 0xaaaaf9dbc42c in media_transport_remove_owner
profiles/audio/transport.c:322
#3 0xaaaaf9dc0b64 in bap_disable_complete
profiles/audio/transport.c:632
#4 0xaaaaf9dc0b64 in release profiles/audio/transport.c:674
#5 0xaaaaf9ff2574 in process_message gdbus/object.c:246
#6 0xffff6e69de78 in _dbus_object_tree_dispatch_and_unlock
../../dbus/dbus-object-tree.c:1021
#7 0xaaaaf9fe44a4 in message_dispatch gdbus/mainloop.c:59
#8 0xffff6e76c444 in g_idle_dispatch ../glib/gmain.c:6282
#9 0xffff6e77030c in g_main_dispatch ../glib/gmain.c:3476
#10 0xffff6e77030c in g_main_context_dispatch_unlocked
../glib/gmain.c:4284
#11 0xffff6e7ce598 in g_main_context_iterate_unlocked.isra.0
../glib/gmain.c:4349
#12 0xffff6e771a60 in g_main_loop_run
(/lib64/libglib-2.0.so.0+0x61a60) (BuildId:
7d17ee836a99abf35136ebb46126a95dee66511a)
#13 0xaaaafa0ad0d8 in mainloop_run src/shared/mainloop-glib.c:66
#14 0xaaaafa0ad920 in mainloop_run_with_signal
src/shared/mainloop-notify.c:188
#15 0xaaaaf9d5489c in main src/main.c:1452
#16 0xffff6dd209d8 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
#17 0xffff6dd20aac in __libc_start_main_impl ../csu/libc-start.c:360
#18 0xaaaaf9d5686c in _start
(/home/jonas/git/bluez/src/bluetoothd+0x55686c) (BuildId:
8922b026a55aac729ed13de54b3a622a31b5bb2b)
previously allocated by thread T0 here:
#0 0xffff6ea25218 in calloc (/lib64/libasan.so.8+0xc5218) (BuildId:
a77e9fa1e1448d41e9a8ddaf28aa5612f3ffc397)
#1 0xffff6e7769e4 in g_malloc0 (/lib64/libglib-2.0.so.0+0x669e4)
(BuildId: 7d17ee836a99abf35136ebb46126a95dee66511a)
#2 0xaaaaf9db89c4 in media_owner_create profiles/audio/transport.c:514
#3 0xaaaaf9dbd760 in acquire profiles/audio/transport.c:552
#4 0xaaaaf9dbd760 in acquire profiles/audio/transport.c:538
#5 0xaaaaf9ff2574 in process_message gdbus/object.c:246
#6 0xffff6e69de78 in _dbus_object_tree_dispatch_and_unlock
../../dbus/dbus-object-tree.c:1021
#7 0xaaaaf9fe44a4 in message_dispatch gdbus/mainloop.c:59
#8 0xffff6e76c444 in g_idle_dispatch ../glib/gmain.c:6282
#9 0xffff6e77030c in g_main_dispatch ../glib/gmain.c:3476
#10 0xffff6e77030c in g_main_context_dispatch_unlocked
../glib/gmain.c:4284
#11 0xffff6e7ce598 in g_main_context_iterate_unlocked.isra.0
../glib/gmain.c:4349
#12 0xffff6e771a60 in g_main_loop_run
(/lib64/libglib-2.0.so.0+0x61a60) (BuildId:
7d17ee836a99abf35136ebb46126a95dee66511a)
#13 0xaaaafa0ad0d8 in mainloop_run src/shared/mainloop-glib.c:66
#14 0xaaaafa0ad920 in mainloop_run_with_signal
src/shared/mainloop-notify.c:188
#15 0xaaaaf9d5489c in main src/main.c:1452
#16 0xffff6dd209d8 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
#17 0xffff6dd20aac in __libc_start_main_impl ../csu/libc-start.c:360
#18 0xaaaaf9d5686c in _start
(/home/jonas/git/bluez/src/bluetoothd+0x55686c) (BuildId:
8922b026a55aac729ed13de54b3a622a31b5bb2b)
SUMMARY: AddressSanitizer: heap-use-after-free
profiles/audio/transport.c:426 in a2dp_suspend_complete
Hi Silviu, I encountered a use-after-free in the 5.71 release of Bluez which I bisected to this patch. The backtrace is below: #0 0x000055e93dbc8785 in a2dp_suspend_complete (session=<optimized out>, err=0, user_data=0x55e93e432520) at profiles/audio/transport.c:431 #1 0x000055e93dbb97ea in finalize_suspend (data=data@entry=0x55e93e435880) at profiles/audio/a2dp.c:376 #2 0x000055e93dbb98c0 in suspend_cfm (session=0x55e93e4317b0, sep=<optimized out>, stream=<optimized out>, err=0x0, user_data=0x55e93e41e850) at profiles/audio/a2dp.c:1276 #3 0x000055e93dbbfa4b in avdtp_suspend_resp (data=0x55e93e431823, size=<optimized out>, stream=0x55e93e433e60, session=0x55e93e4317b0) at profiles/audio/avdtp.c:2900 #4 avdtp_parse_resp (transaction=<optimized out>, size=<optimized out>, buf=0x55e93e431823, signal_id=<optimized out>, stream=0x55e93e433e60, session=0x55e93e4317b0) at profiles/audio/avdtp.c:2985 #5 session_cb (chan=<optimized out>, cond=<optimized out>, data=0x55e93e4317b0) at profiles/audio/avdtp.c:2286 #6 0x00007f5e225b9f69 in g_main_dispatch (context=0x55e93e3c6800) at ../glib/glib/gmain.c:3476 [...] Originally reported in [1] [1] https://gitlab.archlinux.org/archlinux/packaging/packages/bluez/-/issues/3 Cheers, Ronan
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c index 1e03b7b51..23ea267f6 100644 --- a/profiles/audio/transport.c +++ b/profiles/audio/transport.c @@ -606,11 +606,38 @@ static DBusMessage *try_acquire(DBusConnection *conn, DBusMessage *msg, return NULL; } +static void bap_stop_complete(struct bt_bap_stream *stream, + uint8_t code, uint8_t reason, + void *user_data) +{ + struct media_owner *owner = user_data; + struct media_request *req = owner->pending; + struct media_transport *transport = owner->transport; + + /* Release always succeeds */ + if (req) { + req->id = 0; + media_request_reply(req, 0); + media_owner_remove(owner); + } + + transport_set_state(transport, TRANSPORT_STATE_IDLE); + media_transport_remove_owner(transport); +} + +static void bap_disable_complete(struct bt_bap_stream *stream, + uint8_t code, uint8_t reason, + void *user_data) +{ + bap_stop_complete(stream, code, reason, user_data); +} + static DBusMessage *release(DBusConnection *conn, DBusMessage *msg, void *data) { struct media_transport *transport = data; struct media_owner *owner = transport->owner; + struct bap_transport *bap = transport->data; const char *sender; struct media_request *req; guint id; @@ -642,6 +669,11 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg, req = media_request_create(msg, id); media_owner_add(owner, req); + if (bt_bap_stream_get_type(bap->stream) == + BT_BAP_STREAM_TYPE_BCAST) { + bap_disable_complete(bap->stream, 0x00, 0x00, owner); + } + return NULL; } @@ -1370,32 +1402,6 @@ static guint resume_bap(struct media_transport *transport, return id; } -static void bap_stop_complete(struct bt_bap_stream *stream, - uint8_t code, uint8_t reason, - void *user_data) -{ - struct media_owner *owner = user_data; - struct media_request *req = owner->pending; - struct media_transport *transport = owner->transport; - - /* Release always succeeds */ - if (req) { - req->id = 0; - media_request_reply(req, 0); - media_owner_remove(owner); - } - - transport_set_state(transport, TRANSPORT_STATE_IDLE); - media_transport_remove_owner(transport); -} - -static void bap_disable_complete(struct bt_bap_stream *stream, - uint8_t code, uint8_t reason, - void *user_data) -{ - bap_stop_complete(stream, code, reason, user_data); -} - static guint suspend_bap(struct media_transport *transport, struct media_owner *owner) { @@ -1499,9 +1505,14 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state, return; break; case BT_BAP_STREAM_STATE_STREAMING: - if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SOURCE) + if ((bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SOURCE) || + (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)) bap_update_bcast_qos(transport); break; + case BT_BAP_STREAM_STATE_RELEASING: + if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK) + return; + break; } io = bt_bap_stream_get_io(stream);