[PATCH BlueZ] bap: don't track streams without setup except for ucast server

[PATCH BlueZ] bap: don't track streams without setup except for ucast server

[Pauli Virtanen]

data->streams is is used for determining which streams can connect to
listening socket. This stream list is specific to ucast server.

Rename the variable to data->server_streams, and only put ucast server
streams there.

Fixes data->streams accumulating dead stream pointers.
---
 profiles/audio/bap.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 88d170585..ee7c8bc49 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -106,7 +106,7 @@ struct bap_data {
 	struct queue *snks;
 	struct queue *bcast;
 	struct queue *bcast_snks;
-	struct queue *streams;
+	struct queue *server_streams;
 	GIOChannel *listen_io;
 	unsigned int io_id;
 	int selecting;
@@ -158,7 +158,7 @@ static void bap_data_free(struct bap_data *data)
 	queue_destroy(data->snks, ep_unregister);
 	queue_destroy(data->srcs, ep_unregister);
 	queue_destroy(data->bcast, ep_unregister);
-	queue_destroy(data->streams, NULL);
+	queue_destroy(data->server_streams, NULL);
 	queue_destroy(data->bcast_snks, setup_free);
 	bt_bap_ready_unregister(data->bap, data->ready_id);
 	bt_bap_state_unregister(data->bap, data->state_id);
@@ -1695,7 +1695,7 @@ static void iso_confirm_cb(GIOChannel *io, void *user_data)
 	DBG("ISO: incoming connect from %s (CIG 0x%02x CIS 0x%02x)",
 					address, qos.ucast.cig, qos.ucast.cis);
 
-	stream = queue_remove_if(data->streams, match_stream_qos, &qos);
+	stream = queue_remove_if(data->server_streams, match_stream_qos, &qos);
 	if (!stream) {
 		error("No matching stream found");
 		goto drop;
@@ -2040,6 +2040,12 @@ static void setup_listen_io(struct bap_data *data, struct bt_bap_stream *stream,
 
 	DBG("stream %p", stream);
 
+	if (!data->server_streams)
+		data->server_streams = queue_new();
+
+	if (!queue_find(data->server_streams, NULL, stream))
+		queue_push_tail(data->server_streams, stream);
+
 	/* If IO already set skip creating it again */
 	if (bt_bap_stream_get_io(stream) || data->listen_io)
 		return;
@@ -2143,12 +2149,6 @@ static void setup_create_io(struct bap_data *data, struct bap_setup *setup,
 	DBG("setup %p stream %p defer %s", setup, stream,
 				defer ? "true" : "false");
 
-	if (!data->streams)
-		data->streams = queue_new();
-
-	if (!queue_find(data->streams, NULL, stream))
-		queue_push_tail(data->streams, stream);
-
 	switch (bt_bap_stream_get_type(stream)) {
 	case BT_BAP_STREAM_TYPE_UCAST:
 		setup_create_ucast_io(data, setup, stream, defer);
@@ -2184,7 +2184,7 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
 		if (setup)
 			setup_free(setup);
 		else
-			queue_remove(data->streams, stream);
+			queue_remove(data->server_streams, stream);
 		break;
 	case BT_BAP_STREAM_STATE_CONFIG:
 		if (setup && !setup->id) {
@@ -2365,8 +2365,6 @@ static void bap_state_bcast_src(struct bt_bap_stream *stream, uint8_t old_state,
 		/* Release stream if idle */
 		if (setup)
 			setup_free(setup);
-		else
-			queue_remove(data->streams, stream);
 		break;
 	case BT_BAP_STREAM_STATE_CONFIG:
 		// TO DO Reconfiguration
@@ -2469,8 +2467,6 @@ static void bap_state_bcast_sink(struct bt_bap_stream *stream,
 		/* Release stream if idle */
 		if (setup)
 			setup_free(setup);
-		else
-			queue_remove(data->streams, stream);
 		break;
 	case BT_BAP_STREAM_STATE_CONFIG:
 		if (!setup)
-- 
2.49.0

[PATCH BlueZ] shared/bap: fix crash when removing PAC

[Pauli Virtanen]

When PAC is removed, streams need to go through RELEASING flow, which in
some cases is not immediate. Access to stream->lpac is UAF during this
time, e.g. in profiles/audio/bap.c:bap_find_setup_by_stream

Allow stream->lpac == NULL. This should occur only if stream is
RELEASING.

When releasing streams due to removed PAC, do RELEASING->IDLE as we
can't cache config then.
---
 src/shared/bap.c | 52 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 3a11cb082..4c5b38b1e 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -294,6 +294,7 @@ struct bt_bap_stream {
 	uint8_t state;
 	unsigned int state_id;
 	struct queue *pending_states;
+	bool no_cache_config;
 	bool client;
 	void *user_data;
 };
@@ -1000,6 +1001,9 @@ static void stream_notify_config(struct bt_bap_stream *stream)
 
 	DBG(stream->bap, "stream %p", stream);
 
+	if (!lpac)
+		return;
+
 	len = sizeof(*status) + sizeof(*config) + stream->cc->iov_len;
 	status = malloc(len);
 
@@ -1163,7 +1167,7 @@ static struct bt_bap *bt_bap_ref_safe(struct bt_bap *bap)
 
 static void bap_stream_clear_cfm(struct bt_bap_stream *stream)
 {
-	if (!stream->lpac->ops || !stream->lpac->ops->clear)
+	if (!stream->lpac || !stream->lpac->ops || !stream->lpac->ops->clear)
 		return;
 
 	stream->lpac->ops->clear(stream, stream->lpac->user_data);
@@ -1518,6 +1522,12 @@ static uint8_t stream_config(struct bt_bap_stream *stream, struct iovec *cc,
 
 	DBG(stream->bap, "stream %p", stream);
 
+	if (!pac) {
+		ascs_ase_rsp_add(rsp, stream->ep->id, BT_ASCS_RSP_CONF_REJECTED,
+							BT_ASCS_REASON_CODEC);
+		return 0;
+	}
+
 	/* TODO: Wait for pac->ops response */
 	ascs_ase_rsp_success(rsp, stream->ep->id);
 
@@ -1962,6 +1972,9 @@ static unsigned int bap_bcast_config(struct bt_bap_stream *stream,
 				     struct bt_bap_qos *qos, struct iovec *data,
 				     bt_bap_stream_func_t func, void *user_data)
 {
+	if (!stream->lpac)
+		return 0;
+
 	stream->qos = *qos;
 	stream->lpac->ops->config(stream, stream->cc, &stream->qos,
 			ep_config_cb, stream->lpac->user_data);
@@ -2201,18 +2214,23 @@ static uint8_t stream_release(struct bt_bap_stream *stream, struct iovec *rsp)
 	 * to take action immeditely.
 	 */
 	if (!stream->io) {
+		bool cache_config = !stream->no_cache_config;
+
 		switch (bt_bap_stream_get_state(stream)) {
 		case BT_BAP_STREAM_STATE_CONFIG:
 			/* Released (no caching) */
-			stream_set_state(stream, BT_BAP_STREAM_STATE_RELEASING);
-			stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
+			cache_config = false;
 			break;
 		default:
 			/* Released (caching) */
-			stream_set_state(stream, BT_BAP_STREAM_STATE_RELEASING);
-			stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
 			break;
 		}
+
+		stream_set_state(stream, BT_BAP_STREAM_STATE_RELEASING);
+		if (cache_config)
+			stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
+		else
+			stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
 	} else
 		stream_set_state(stream, BT_BAP_STREAM_STATE_RELEASING);
 
@@ -4214,15 +4232,23 @@ static bool match_stream_lpac(const void *data, const void *user_data)
 	return stream->lpac == pac;
 }
 
-static void remove_streams(void *data, void *user_data)
+static void remove_lpac_streams(void *data, void *user_data)
 {
 	struct bt_bap *bap = data;
 	struct bt_bap_pac *pac = user_data;
 	struct bt_bap_stream *stream;
 
-	stream = queue_remove_if(bap->streams, match_stream_lpac, pac);
-	if (stream)
+	while (1) {
+		stream = queue_remove_if(bap->streams, match_stream_lpac, pac);
+		if (!stream)
+			break;
+
+		bt_bap_stream_ref(stream);
+		stream->no_cache_config = true;
 		bt_bap_stream_release(stream, NULL, NULL);
+		stream->lpac = NULL;
+		bt_bap_stream_unref(stream);
+	}
 }
 
 static void bap_pac_sink_removed(void *data, void *user_data)
@@ -4277,7 +4303,7 @@ bool bt_bap_remove_pac(struct bt_bap_pac *pac)
 	return false;
 
 found:
-	queue_foreach(sessions, remove_streams, pac);
+	queue_foreach(sessions, remove_lpac_streams, pac);
 	queue_foreach(sessions, notify_session_pac_removed, pac);
 	bap_pac_free(pac);
 	return true;
@@ -4998,7 +5024,7 @@ static void bap_stream_config_cfm(struct bt_bap_stream *stream)
 {
 	int err;
 
-	if (!stream->lpac->ops || !stream->lpac->ops->config)
+	if (!stream->lpac || !stream->lpac->ops || !stream->lpac->ops->config)
 		return;
 
 	err = stream->lpac->ops->config(stream, stream->cc, &stream->qos,
@@ -6409,6 +6435,9 @@ bool bt_bap_match_bcast_sink_stream(const void *data, const void *user_data)
 {
 	const struct bt_bap_stream *stream = data;
 
+	if (!stream->lpac)
+		return false;
+
 	return stream->lpac->type == BT_BAP_BCAST_SINK;
 }
 
@@ -6845,6 +6874,9 @@ static void add_new_subgroup(struct bt_base *base,
 	uint16_t cid = 0;
 	uint16_t vid = 0;
 
+	if (!lpac)
+		return;
+
 	bt_bap_pac_get_vendor_codec(lpac, &sgrp->codec.id, &cid,
 			&vid, NULL, NULL);
 	sgrp->codec.cid = cid;
-- 
2.49.0

Re: [PATCH BlueZ] shared/bap: fix crash when removing PAC

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Mon, 21 Apr 2025 19:30:54 +0300 you wrote:
> When PAC is removed, streams need to go through RELEASING flow, which in
> some cases is not immediate. Access to stream->lpac is UAF during this
> time, e.g. in profiles/audio/bap.c:bap_find_setup_by_stream
> 
> Allow stream->lpac == NULL. This should occur only if stream is
> RELEASING.
> 
> [...]

Here is the summary with links:
  - [BlueZ] shared/bap: fix crash when removing PAC
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=578a6fd688b0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH BlueZ] bap: don't track streams without setup except for ucast server

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Mon, 21 Apr 2025 19:30:53 +0300 you wrote:
> data->streams is is used for determining which streams can connect to
> listening socket. This stream list is specific to ucast server.
> 
> Rename the variable to data->server_streams, and only put ucast server
> streams there.
> 
> Fixes data->streams accumulating dead stream pointers.
> 
> [...]

Here is the summary with links:
  - [BlueZ] bap: don't track streams without setup except for ucast server
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d5ef57305b79

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html