Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-4689-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Bastien Nocera <hadess@hadess.net>
To: linux-bluetooth@vger.kernel.org
Cc: Bastien Nocera <hadess@hadess.net>
Subject: [BlueZ 08/15] shared/bap: Fix possible use-after-free
Date: Thu, 16 May 2024 11:03:12 +0200
Message-ID: <20240516090340.61417-9-hadess@hadess.net>
In-Reply-To: <20240516090340.61417-1-hadess@hadess.net>
References: <20240516090340.61417-1-hadess@hadess.net>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

stream_set_state() might call bap_stream_detach() if the stream is in
the process of being detached, causing a use-after-free.

Return false from stream_set_state() if the stream is unsafe to
manipulate (ie. was in the process of being detached and freed).

Error: USE_AFTER_FREE (CWE-416): [#def37] [important]
bluez-5.75/src/shared/bap.c:2490:2: freed_arg: "stream_set_state" frees "stream".
bluez-5.75/src/shared/bap.c:2493:2: deref_after_free: Dereferencing freed pointer "stream".
2491|
2492|		/* Sink can autonomously for to Streaming state if io already exits */
2493|->		if (stream->io && stream->ep->dir == BT_BAP_SINK)
2494|			stream_set_state(stream, BT_BAP_STREAM_STATE_STREAMING);
2495|
---
 src/shared/bap.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 1316d7c73d02..0026bc8dc989 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -1298,7 +1298,8 @@ static void bap_stream_state_changed(struct bt_bap_stream *stream)
 	}
 }
 
-static void stream_set_state(struct bt_bap_stream *stream, uint8_t state)
+/* Return false if the stream is being detached */
+static bool stream_set_state(struct bt_bap_stream *stream, uint8_t state)
 {
 	struct bt_bap *bap = stream->bap;
 
@@ -1308,13 +1309,14 @@ static void stream_set_state(struct bt_bap_stream *stream, uint8_t state)
 	bap = bt_bap_ref_safe(bap);
 	if (!bap) {
 		bap_stream_detach(stream);
-		return;
+		return false;
 	}
 
 	if (stream->ops && stream->ops->set_state)
 		stream->ops->set_state(stream, state);
 
 	bt_bap_unref(bap);
+	return true;
 }
 
 static void ep_config_cb(struct bt_bap_stream *stream, int err)
@@ -2487,7 +2489,8 @@ static uint8_t stream_enable(struct bt_bap_stream *stream, struct iovec *meta,
 	util_iov_free(stream->meta, 1);
 	stream->meta = util_iov_dup(meta, 1);
 
-	stream_set_state(stream, BT_BAP_STREAM_STATE_ENABLING);
+	if (!stream_set_state(stream, BT_BAP_STREAM_STATE_ENABLING))
+		return 1;
 
 	/* Sink can autonomously for to Streaming state if io already exits */
 	if (stream->io && stream->ep->dir == BT_BAP_SINK)
-- 
2.44.0