[PATCH BlueZ v1 1/3] bass: Fix possible crash on bass_update_bis_sync

[PATCH BlueZ v1 1/3] bass: Fix possible crash on bass_update_bis_sync

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

bass_update_bis_sync does use bass_remove_bis which may end up
removing the current entry causing a crash on entry->next, to avoid
that prefetch the next entry.
---
 profiles/audio/bass.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/bass.c b/profiles/audio/bass.c
index a5ef80fbc835..533d45babfeb 100644
--- a/profiles/audio/bass.c
+++ b/profiles/audio/bass.c
@@ -2014,11 +2014,15 @@ static void bass_update_bis_sync(struct bass_delegator *dg,
 	const struct queue_entry *entry;
 
 	/* Check if existing setups if BIS needs to be added/removed */
-	for (entry = queue_get_entries(dg->setups); entry;
-				entry = entry->next) {
+	for (entry = queue_get_entries(dg->setups); entry;) {
 		struct bass_setup *setup = entry->data;
 		uint8_t state;
 
+		/* Prefetch next entry since the likes of bass_remove_bis can
+		 * end up removing the next entry.
+		 */
+		entry = entry->next;
+
 		state = bt_bap_stream_get_state(setup->stream);
 
 		DBG("stream %p: BIS %d state %s(%u)", setup->stream, setup->bis,
-- 
2.54.0

[PATCH BlueZ v1 2/3] shared/bap: Check if stream is valid before attempting to release

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

bt_bap_stream_release shall check if the stream is still valid before
attempting to release it just as done with other operations.
---
 src/shared/bap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 6f2f4fc11f7c..9dd07bc5f2e2 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -6727,6 +6727,9 @@ unsigned int bt_bap_stream_release(struct bt_bap_stream *stream,
 	unsigned int id;
 	struct bt_bap *bap;
 
+	if (!bap_stream_valid(stream))
+		return 0;
+
 	if (!stream || !stream->ops || !stream->ops->release)
 		return 0;
 
-- 
2.54.0

[PATCH BlueZ v1 3/3] shared/bap: Don't transition to IDLE inside bap_bcast_set_state

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Remove the recursive stream_set_state(IDLE) call from the RELEASING
case in bap_bcast_set_state. This call re-entered bap_bcast_set_state
while the state_cbs queue was still being iterated, causing a
use-after-free if a callback unregistered itself during notification.
---
 src/shared/bap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 9dd07bc5f2e2..6086924a9cb7 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -2436,7 +2436,6 @@ static void bap_bcast_set_state(struct bt_bap_stream *stream, uint8_t state)
 		break;
 	case BT_ASCS_ASE_STATE_RELEASING:
 		bap_stream_io_detach(stream);
-		stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
 		break;
 	case BT_ASCS_ASE_STATE_ENABLING:
 		if (bt_bap_stream_get_io(stream))
@@ -2579,6 +2578,7 @@ static unsigned int bap_bcast_release(struct bt_bap_stream *stream,
 					void *user_data)
 {
 	stream_set_state(stream, BT_BAP_STREAM_STATE_RELEASING);
+	stream_set_state(stream, BT_BAP_STREAM_STATE_IDLE);
 
 	return 1;
 }
-- 
2.54.0

RE: [BlueZ,v1,1/3] bass: Fix possible crash on bass_update_bis_sync

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      2.77 seconds
GitLint                       PASS      1.47 seconds
BuildEll                      PASS      20.57 seconds
BluezMake                     PASS      658.38 seconds
MakeCheck                     PASS      14.89 seconds
MakeDistcheck                 PASS      249.54 seconds
CheckValgrind                 PASS      278.40 seconds
CheckSmatch                   WARNING   354.17 seconds
bluezmakeextell               PASS      184.03 seconds
IncrementalBuild              PASS      701.98 seconds
ScanBuild                     PASS      1032.34 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures


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

---
Regards,
Linux Bluetooth