RE: Update transport acquire/release flow BAP bcast source

RE: Update transport acquire/release flow BAP bcast source

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      1.46 seconds
GitLint                       PASS      0.89 seconds
BuildEll                      PASS      28.53 seconds
BluezMake                     PASS      885.38 seconds
MakeCheck                     PASS      12.39 seconds
MakeDistcheck                 PASS      162.32 seconds
CheckValgrind                 PASS      261.73 seconds
CheckSmatch                   PASS      352.83 seconds
bluezmakeextell               PASS      108.54 seconds
IncrementalBuild              PASS      2179.90 seconds
ScanBuild                     PASS      1078.14 seconds



---
Regards,
Linux Bluetooth

[PATCH v3 0/2] Update transport acquire/release flow BAP bcast source

[Silviu Florian Barbulescu]

Update transport acquire/release flow for BAP broadcast source to 
fix failure on transport release, and second acquire

Silviu Florian Barbulescu (2):
  transport: Update transport release flow for broadcast source
  bap:Update transport acquire/release flow for bcast source

 profiles/audio/bap.c       | 51 ++++++++++++++++++++++++++----
 profiles/audio/transport.c | 65 ++++++++++++++++++++++----------------
 2 files changed, 82 insertions(+), 34 deletions(-)


base-commit: 7a79ff88fd9576cd9935a8c0bab4906b7c38ea03
-- 
2.39.2

[PATCH v3 1/2] transport: Update transport release flow for bcast src

[Silviu Florian Barbulescu]

Update transport release flow for broadcast source

---
 profiles/audio/transport.c | 65 ++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 27 deletions(-)

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);
-- 
2.39.2

[PATCH v3 2/2] bap:Update transport acquire/release flow for bcast src

[Silviu Florian Barbulescu]

Update transport acquire/release flow for BAP bcast source

---
 profiles/audio/bap.c | 51 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 7 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 48a1a4f86..f23e04f00 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -1312,6 +1312,32 @@ static struct bap_ep *bap_find_ep_by_stream(struct bap_data *data,
 	return NULL;
 }
 
+static void iso_connect_bcast_cb(GIOChannel *chan, GError *err,
+					gpointer user_data)
+{
+	struct bt_bap_stream *stream = user_data;
+	int fd;
+
+	if (err) {
+		error("%s", err->message);
+		bt_bap_stream_set_io(stream, -1);
+		return;
+	}
+
+	DBG("ISO connected");
+
+	fd = g_io_channel_unix_get_fd(chan);
+
+	if (bt_bap_stream_set_io(stream, fd)) {
+		bt_bap_stream_start(stream, NULL, NULL);
+		g_io_channel_set_close_on_unref(chan, FALSE);
+		return;
+	}
+
+	error("Unable to set IO");
+	bt_bap_stream_set_io(stream, -1);
+}
+
 static void iso_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 {
 	struct bt_bap_stream *stream = user_data;
@@ -1560,6 +1586,17 @@ static gboolean bap_io_disconnected(GIOChannel *io, GIOCondition cond,
 	return FALSE;
 }
 
+static void bap_connect_bcast_io_cb(GIOChannel *chan, GError *err,
+					gpointer user_data)
+{
+	struct bap_ep *ep = user_data;
+
+	if (!ep->stream)
+		return;
+
+	iso_connect_bcast_cb(chan, err, ep->stream);
+}
+
 static void bap_connect_io_cb(GIOChannel *chan, GError *err, gpointer user_data)
 {
 	struct bap_ep *ep = user_data;
@@ -1661,7 +1698,7 @@ static void bap_connect_io_broadcast(struct bap_data *data, struct bap_ep *ep,
 	DBG("ep %p stream %p ", ep, stream);
 	ba2str(btd_adapter_get_address(adapter), addr);
 
-	io = bt_io_connect(bap_connect_io_cb, ep, NULL, &err,
+	io = bt_io_connect(bap_connect_bcast_io_cb, ep, NULL, &err,
 			BT_IO_OPT_SOURCE_BDADDR,
 			btd_adapter_get_address(adapter),
 			BT_IO_OPT_DEST_BDADDR,
@@ -1887,18 +1924,16 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
 		}
 		break;
 	case BT_BAP_STREAM_STATE_QOS:
-		bap_create_io(data, ep, stream, true);
+		if (bt_bap_stream_get_type(stream) ==
+					BT_BAP_STREAM_TYPE_UCAST) {
+			bap_create_io(data, ep, stream, true);
+		}
 		break;
 	case BT_BAP_STREAM_STATE_ENABLING:
 		if (ep)
 			bap_create_io(data, ep, stream, false);
 		break;
 	case BT_BAP_STREAM_STATE_STREAMING:
-		if (bt_bap_stream_get_type(stream) ==
-				BT_BAP_STREAM_TYPE_BCAST) {
-			if (ep)
-				bap_create_io(data, ep, stream, false);
-		}
 		break;
 	}
 }
@@ -2116,6 +2151,8 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
 
 			ep->qos.bcast.big = qos.bcast.big;
 			ep->qos.bcast.bis = qos.bcast.bis;
+			bt_bap_stream_config(ep->stream, &ep->qos,
+					ep->caps, NULL, NULL);
 		}
 
 		DBG("stream %p fd %d: BIG 0x%02x BIS 0x%02x", stream, fd,
-- 
2.39.2

RE: Update transport acquire/release flow BAP bcast source

[bluez.test.bot]

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

RE: Update transport acquire/release flow BAP bcast source

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      0.94 seconds
GitLint                       PASS      0.55 seconds
BuildEll                      PASS      29.34 seconds
BluezMake                     PASS      1032.60 seconds
MakeCheck                     PASS      12.40 seconds
MakeDistcheck                 PASS      164.78 seconds
CheckValgrind                 PASS      269.16 seconds
CheckSmatch                   PASS      361.45 seconds
bluezmakeextell               PASS      110.17 seconds
IncrementalBuild              PASS      1763.22 seconds
ScanBuild                     PASS      1091.40 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH v3 1/2] transport: Update transport release flow for bcast src

[Jonas Dreßler]

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

Re: [PATCH v3 1/2] transport: Update transport release flow for bcast src

[Ronan Pigott]

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