All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request
@ 2026-08-21 19:34 George Kiagiadakis
  2026-08-21 19:34 ` [PATCH BlueZ 1/5] player: Fix crash on MediaItem1.Play() without a browsing scope George Kiagiadakis
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: George Kiagiadakis @ 2026-08-21 19:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: George Kiagiadakis

A player that advertises the AVRCP NowPlaying feature bit but not the
Browsing bit exports playable MediaItem1 objects while the player scope
is still unset, and calling org.bluez.MediaItem1.Play() on one of them
crashes bluetoothd with a NULL dereference at offset 0x28.

media_item_play() dereferences mp->scope, which is only ever set from
SetBrowsedPlayer or ChangeFolder, and both are reached only when the
player advertises Browsing (features[7] & 0x08). The /NowPlaying folder
and its playable items are gated on a different bit (features[8] &
0x02), so the two can disagree. msg sits at offset 40 in struct
media_folder on LP64, which is the reported fault address.

Patch 1 fixes the crash. A plain NULL check is not enough, because the
pending message would then have nowhere to live and Play() would answer
nothing at all rather than crash. The pending request moves from struct
media_folder to struct media_player instead. Every user already stored
into mp->scope->msg, so the slot was per player in all but name, and the
mutual exclusion between ListItems, Search, ChangeFolder and Play is
preserved. The move also fixes a lost reply, since each completion
re-read mp->scope and found a different folder whenever avrcp moved the
scope while a request was in flight.

Patches 2 to 4 are further defects in the same area, found while
auditing the ownership of that message:

  - destroying a player dropped the pending request without answering
    it, so the caller waited out its D-Bus timeout on every AVRCP
    disconnect;

  - NumberOfItems has not been refreshed on SetBrowsedPlayer since
    f17d3a2c3, because a guard swallows the property update that
    commit deferred into the completion;

  - a busy Search() reports EINVAL where its three siblings report
    EBUSY.

Patch 5 adds unit/test-media-player, which drives the D-Bus surface of
profiles/audio/player.c over a private session bus. Against the tree
before this series, three of its nine tests crash and three fail.

Each patch builds and passes make check on its own. The final tree
passes 39/39 and is clean under valgrind.

George Kiagiadakis (5):
  player: Fix crash on MediaItem1.Play() without a browsing scope
  player: Answer pending request when the player is destroyed
  player: Fix NumberOfItems never being updated on SetBrowsedPlayer
  player: Report EBUSY from a busy Search()
  unit/test-media-player: Add media player tests

 .gitignore               |   1 +
 Makefile.am              |  13 +
 profiles/audio/player.c  |  80 ++--
 unit/test-media-player.c | 838 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 891 insertions(+), 41 deletions(-)
 create mode 100644 unit/test-media-player.c


base-commit: c73fa2f9ae2d366cb8a4f101fa9a5ccd9f33a4ea
-- 
2.54.0 (Apple Git-157)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH BlueZ 1/5] player: Fix crash on MediaItem1.Play() without a browsing scope
  2026-08-21 19:34 [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request George Kiagiadakis
@ 2026-08-21 19:34 ` George Kiagiadakis
  2026-08-21 20:38   ` player: Fix crash and related defects around the pending request bluez.test.bot
  2026-08-21 19:34 ` [PATCH BlueZ 2/5] player: Answer pending request when the player is destroyed George Kiagiadakis
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 8+ messages in thread
From: George Kiagiadakis @ 2026-08-21 19:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: George Kiagiadakis

media_item_play() dereferenced mp->scope without checking it:

	struct media_folder *folder = mp->scope;
	...
	if (folder->msg)

mp->scope is only ever set by media_player_set_scope(), which is reached
from media_player_set_folder() (SetBrowsedPlayer) and from
media_player_set_folder_by_uid() (ChangeFolder). Both of those require
the player to advertise the Browsing feature bit, since
avrcp_player_parse_features() only creates /Filesystem when
features[7] & 0x08 is set.

The /NowPlaying folder is gated on a different bit, features[8] & 0x02,
and media_player_set_playlist_item() registers its items as playable
MediaItem1 objects regardless of the scope. A player that reports
NowPlaying but not Browsing therefore exports playable items while
mp->scope is still NULL, and calling Play() on one of them crashes
bluetoothd. msg sits at offset 40 in struct media_folder on LP64, which
matches the reported "segfault at 28".

A plain NULL check is not enough: the pending message has nowhere to be
stored, so Play() would answer nothing at all and the caller would hang
instead of crashing. Move the pending request from struct media_folder
to struct media_player instead. Every user of folder->msg already stored
into mp->scope->msg, so the slot was per player in all but name, and
moving it keeps the existing mutual exclusion between ListItems, Search,
ChangeFolder and Play intact.

Playing now works rather than merely not crashing: ct_play_item() picks
the AVRCP scope from the path, so an item under /NowPlaying is played
with the Now Playing scope (0x03).

Moving the message off the folder also fixes a lost reply. Each
completion re-read mp->scope, so when avrcp moved the scope while a
request was pending, for instance on SetBrowsedPlayer, the completion
found a different folder with a NULL msg, returned early and left the
D-Bus caller without an answer.

Fixes: 43b0855abdf4 ("audio/player: Report PlayItem errors")

Assisted-by: Claude:claude-opus-5 valgrind
---
 profiles/audio/player.c | 75 +++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 40 deletions(-)

diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 3af9a1824..568c70770 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -66,7 +66,6 @@ struct media_folder {
 	uint32_t		number_of_items;/* Number of items */
 	GSList			*subfolders;
 	GSList			*items;
-	DBusMessage		*msg;
 };
 
 struct media_player {
@@ -89,6 +88,7 @@ struct media_player {
 	struct player_callback	*cb;
 	GSList			*pending;
 	GSList			*folders;
+	DBusMessage		*msg;		/* Pending request */
 	uint16_t		obex_port;
 };
 
@@ -662,19 +662,18 @@ static void parse_folder_list(gpointer data, gpointer user_data)
 void media_player_list_complete(struct media_player *mp, GSList *items,
 								int err)
 {
-	struct media_folder *folder = mp->scope;
 	DBusMessage *reply;
 	DBusMessageIter iter, array;
 
-	if (folder == NULL || folder->msg == NULL)
+	if (mp->msg == NULL)
 		return;
 
 	if (err < 0) {
-		reply = btd_error_failed(folder->msg, strerror(-err));
+		reply = btd_error_failed(mp->msg, strerror(-err));
 		goto done;
 	}
 
-	reply = dbus_message_new_method_return(folder->msg);
+	reply = dbus_message_new_method_return(mp->msg);
 
 	dbus_message_iter_init_append(reply, &iter);
 
@@ -694,8 +693,8 @@ void media_player_list_complete(struct media_player *mp, GSList *items,
 
 done:
 	g_dbus_send_message(btd_get_dbus_connection(), reply);
-	dbus_message_unref(folder->msg);
-	folder->msg = NULL;
+	dbus_message_unref(mp->msg);
+	mp->msg = NULL;
 }
 
 static struct media_item *
@@ -719,15 +718,14 @@ media_player_create_subfolder(struct media_player *mp, const char *name,
 
 void media_player_search_complete(struct media_player *mp, int ret)
 {
-	struct media_folder *folder = mp->scope;
 	struct media_folder *search = mp->search;
 	DBusMessage *reply;
 
-	if (folder == NULL || folder->msg == NULL)
+	if (mp->msg == NULL)
 		return;
 
 	if (ret < 0) {
-		reply = btd_error_failed(folder->msg, strerror(-ret));
+		reply = btd_error_failed(mp->msg, strerror(-ret));
 		goto done;
 	}
 
@@ -740,14 +738,14 @@ void media_player_search_complete(struct media_player *mp, int ret)
 
 	search->number_of_items = ret;
 
-	reply = g_dbus_create_reply(folder->msg,
+	reply = g_dbus_create_reply(mp->msg,
 				DBUS_TYPE_OBJECT_PATH, &search->item->path,
 				DBUS_TYPE_INVALID);
 
 done:
 	g_dbus_send_message(btd_get_dbus_connection(), reply);
-	dbus_message_unref(folder->msg);
-	folder->msg = NULL;
+	dbus_message_unref(mp->msg);
+	mp->msg = NULL;
 }
 
 void media_player_total_items_complete(struct media_player *mp,
@@ -755,7 +753,7 @@ void media_player_total_items_complete(struct media_player *mp,
 {
 	struct media_folder *folder = mp->scope;
 
-	if (folder == NULL || folder->msg == NULL)
+	if (folder == NULL || mp->msg == NULL)
 		return;
 
 	if (folder->number_of_items != num_of_items) {
@@ -827,14 +825,14 @@ static DBusMessage *media_folder_search(DBusConnection *conn, DBusMessage *msg,
 	if (!mp->searchable || folder != mp->folder || !cb->cbs->search)
 		return btd_error_not_supported(msg);
 
-	if (folder->msg != NULL)
+	if (mp->msg != NULL)
 		return btd_error_failed(msg, strerror(EINVAL));
 
 	err = cb->cbs->search(mp, string, cb->user_data);
 	if (err < 0)
 		return btd_error_failed(msg, strerror(-err));
 
-	folder->msg = dbus_message_ref(msg);
+	mp->msg = dbus_message_ref(msg);
 
 	return NULL;
 }
@@ -911,7 +909,7 @@ static DBusMessage *media_folder_list_items(DBusConnection *conn,
 	if (cb->cbs->list_items == NULL)
 		return btd_error_not_supported(msg);
 
-	if (folder->msg != NULL)
+	if (mp->msg != NULL)
 		return btd_error_failed(msg, strerror(EBUSY));
 
 	err = cb->cbs->list_items(mp, folder->item->name, start, end,
@@ -919,7 +917,7 @@ static DBusMessage *media_folder_list_items(DBusConnection *conn,
 	if (err < 0)
 		return btd_error_failed(msg, strerror(-err));
 
-	folder->msg = dbus_message_ref(msg);
+	mp->msg = dbus_message_ref(msg);
 
 	return NULL;
 }
@@ -953,9 +951,6 @@ static void media_folder_destroy(void *data)
 	g_slist_free_full(folder->subfolders, media_folder_destroy);
 	g_slist_free_full(folder->items, media_item_destroy);
 
-	if (folder->msg != NULL)
-		dbus_message_unref(folder->msg);
-
 	media_item_destroy(folder->item);
 	g_free(folder);
 }
@@ -1041,7 +1036,7 @@ static DBusMessage *media_folder_change_folder(DBusConnection *conn,
 						DBusMessage *msg, void *data)
 {
 	struct media_player *mp = data;
-	struct media_folder *folder = mp->scope;
+	struct media_folder *folder;
 	struct player_callback *cb = mp->cb;
 	const char *path;
 	int err;
@@ -1051,7 +1046,7 @@ static DBusMessage *media_folder_change_folder(DBusConnection *conn,
 					DBUS_TYPE_INVALID))
 		return btd_error_invalid_args(msg);
 
-	if (folder->msg != NULL)
+	if (mp->msg != NULL)
 		return btd_error_failed(msg, strerror(EBUSY));
 
 	folder = media_player_find_folder(mp, path);
@@ -1083,7 +1078,7 @@ static DBusMessage *media_folder_change_folder(DBusConnection *conn,
 	if (err < 0)
 		return btd_error_failed(msg, strerror(-err));
 
-	mp->scope->msg = dbus_message_ref(msg);
+	mp->msg = dbus_message_ref(msg);
 
 	return NULL;
 }
@@ -1224,25 +1219,24 @@ void media_player_change_folder_complete(struct media_player *mp,
 						const char *path, uint64_t uid,
 						int ret)
 {
-	struct media_folder *folder = mp->scope;
 	DBusMessage *reply;
 
-	if (folder == NULL || folder->msg == NULL)
+	if (mp->msg == NULL)
 		return;
 
 	if (ret < 0) {
-		reply = btd_error_failed(folder->msg, strerror(-ret));
+		reply = btd_error_failed(mp->msg, strerror(-ret));
 		goto done;
 	}
 
 	media_player_set_folder_by_uid(mp, uid, ret);
 
-	reply = g_dbus_create_reply(folder->msg, DBUS_TYPE_INVALID);
+	reply = g_dbus_create_reply(mp->msg, DBUS_TYPE_INVALID);
 
 done:
 	g_dbus_send_message(btd_get_dbus_connection(), reply);
-	dbus_message_unref(folder->msg);
-	folder->msg = NULL;
+	dbus_message_unref(mp->msg);
+	mp->msg = NULL;
 }
 
 void media_player_destroy(struct media_player *mp)
@@ -1263,6 +1257,9 @@ void media_player_destroy(struct media_player *mp)
 						mp->path,
 						MEDIA_FOLDER_INTERFACE);
 
+	if (mp->msg)
+		dbus_message_unref(mp->msg);
+
 	g_slist_free_full(mp->pending, g_free);
 	g_slist_free_full(mp->folders, media_folder_destroy);
 
@@ -1613,7 +1610,6 @@ static DBusMessage *media_item_play(DBusConnection *conn, DBusMessage *msg,
 {
 	struct media_item *item = data;
 	struct media_player *mp = item->player;
-	struct media_folder *folder = mp->scope;
 	struct player_callback *cb = mp->cb;
 	const char *path;
 	int err;
@@ -1621,16 +1617,16 @@ static DBusMessage *media_item_play(DBusConnection *conn, DBusMessage *msg,
 	if (!item->playable || !cb->cbs->play_item)
 		return btd_error_not_supported(msg);
 
-	if (folder->msg)
+	if (mp->msg)
 		return btd_error_failed(msg, strerror(EBUSY));
 
-	path = mp->search && folder == mp->search ? "/Search" : item->path;
+	path = mp->search && mp->scope == mp->search ? "/Search" : item->path;
 
 	err = cb->cbs->play_item(mp, path, item->uid, cb->user_data);
 	if (err < 0)
 		return btd_error_failed(msg, strerror(-err));
 
-	folder->msg = dbus_message_ref(msg);
+	mp->msg = dbus_message_ref(msg);
 
 	return NULL;
 }
@@ -1839,23 +1835,22 @@ static const GDBusPropertyTable media_item_properties[] = {
 
 void media_player_play_item_complete(struct media_player *mp, int err)
 {
-	struct media_folder *folder = mp->scope;
 	DBusMessage *reply;
 
-	if (folder == NULL || folder->msg == NULL)
+	if (mp->msg == NULL)
 		return;
 
 	if (err < 0) {
-		reply = btd_error_failed(folder->msg, strerror(-err));
+		reply = btd_error_failed(mp->msg, strerror(-err));
 		goto done;
 	}
 
-	reply = g_dbus_create_reply(folder->msg, DBUS_TYPE_INVALID);
+	reply = g_dbus_create_reply(mp->msg, DBUS_TYPE_INVALID);
 
 done:
 	g_dbus_send_message(btd_get_dbus_connection(), reply);
-	dbus_message_unref(folder->msg);
-	folder->msg = NULL;
+	dbus_message_unref(mp->msg);
+	mp->msg = NULL;
 }
 
 void media_item_set_playable(struct media_item *item, bool value)
-- 
2.54.0 (Apple Git-157)


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH BlueZ 2/5] player: Answer pending request when the player is destroyed
  2026-08-21 19:34 [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request George Kiagiadakis
  2026-08-21 19:34 ` [PATCH BlueZ 1/5] player: Fix crash on MediaItem1.Play() without a browsing scope George Kiagiadakis
@ 2026-08-21 19:34 ` George Kiagiadakis
  2026-08-21 19:34 ` [PATCH BlueZ 3/5] player: Fix NumberOfItems never being updated on SetBrowsedPlayer George Kiagiadakis
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: George Kiagiadakis @ 2026-08-21 19:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: George Kiagiadakis

media_player_destroy() dropped its reference to the pending request
without answering it. A client with a ListItems(), Search(),
ChangeFolder() or Play() in flight was therefore left waiting for its
own D-Bus timeout to expire, 25s by default, whenever the player went
away. That happens on every AVRCP disconnect, since avrcp destroys the
controller player from its disconnect path.

Reply with org.bluez.Error.Failed instead.

Answering after the g_dbus_unregister_interface() calls above is fine,
as replies are matched by serial rather than by object path, so the
unref site does not need to move.

Assisted-by: Claude:claude-opus-5 valgrind
---
 profiles/audio/player.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 568c70770..fa3810a7f 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -1257,8 +1257,11 @@ void media_player_destroy(struct media_player *mp)
 						mp->path,
 						MEDIA_FOLDER_INTERFACE);
 
-	if (mp->msg)
+	if (mp->msg) {
+		g_dbus_send_message(btd_get_dbus_connection(),
+				btd_error_failed(mp->msg, "Player removed"));
 		dbus_message_unref(mp->msg);
+	}
 
 	g_slist_free_full(mp->pending, g_free);
 	g_slist_free_full(mp->folders, media_folder_destroy);
-- 
2.54.0 (Apple Git-157)


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH BlueZ 3/5] player: Fix NumberOfItems never being updated on SetBrowsedPlayer
  2026-08-21 19:34 [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request George Kiagiadakis
  2026-08-21 19:34 ` [PATCH BlueZ 1/5] player: Fix crash on MediaItem1.Play() without a browsing scope George Kiagiadakis
  2026-08-21 19:34 ` [PATCH BlueZ 2/5] player: Answer pending request when the player is destroyed George Kiagiadakis
@ 2026-08-21 19:34 ` George Kiagiadakis
  2026-08-21 19:34 ` [PATCH BlueZ 4/5] player: Report EBUSY from a busy Search() George Kiagiadakis
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: George Kiagiadakis @ 2026-08-21 19:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: George Kiagiadakis

media_player_total_items_complete() discarded the count reported by the
player unless a D-Bus request happened to be pending:

	if (folder == NULL || folder->msg == NULL)
		return;

Of the paths reaching it, only media_player_change_folder_complete()
still holds a pending message. The count was therefore applied on
ChangeFolder and dropped everywhere else, notably on
media_player_set_folder(), which avrcp calls on SetBrowsedPlayer, that
is precisely when the count is first learned.

The guard reads as copy-paste from the four *_complete() functions
above it. Those need a pending message because they send a reply. This
one only refreshes a property, so there is no request to correlate it
with.

f17d3a2c3 replaced an unconditional emit in media_player_change_scope()
with one deferred into this completion whenever the total_items
callback is present, and the guard then swallowed it. The AVRCP
controller always registers that callback, so NumberOfItems has not
been refreshed on SetBrowsedPlayer since. That commit states the
intent itself: "On response, emit PropertyChanged for 'NumberOfItems'
property".

Note the count is still applied to whatever mp->scope is at completion
time rather than to the folder it was requested for.
media_player_change_scope() sets the scope before asking, so the common
case is right, but a second scope change in flight misattributes it.

Fixes: f17d3a2c3b0d ("audio/avrcp: Add support for GetTotalNumberOfItems")

Assisted-by: Claude:claude-opus-5 valgrind
---
 profiles/audio/player.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index fa3810a7f..7c5ea5b62 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -753,7 +753,7 @@ void media_player_total_items_complete(struct media_player *mp,
 {
 	struct media_folder *folder = mp->scope;
 
-	if (folder == NULL || mp->msg == NULL)
+	if (folder == NULL)
 		return;
 
 	if (folder->number_of_items != num_of_items) {
-- 
2.54.0 (Apple Git-157)


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH BlueZ 4/5] player: Report EBUSY from a busy Search()
  2026-08-21 19:34 [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request George Kiagiadakis
                   ` (2 preceding siblings ...)
  2026-08-21 19:34 ` [PATCH BlueZ 3/5] player: Fix NumberOfItems never being updated on SetBrowsedPlayer George Kiagiadakis
@ 2026-08-21 19:34 ` George Kiagiadakis
  2026-08-21 19:34 ` [PATCH BlueZ 5/5] unit/test-media-player: Add media player tests George Kiagiadakis
  2026-08-24 20:40 ` [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request patchwork-bot+bluetooth
  5 siblings, 0 replies; 8+ messages in thread
From: George Kiagiadakis @ 2026-08-21 19:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: George Kiagiadakis

media_folder_search() answered a request that arrived while another one
was still pending with EINVAL, while media_folder_list_items(),
media_folder_change_folder() and media_item_play() all answer EBUSY for
the very same condition.

The commit that added Search copied the error code from the argument
check sitting directly above it, rather than from ChangeFolder, which
had gained the identical busy check four days earlier and used EBUSY.

Fixes: 0a232a434d4b ("audio/player: Add implementation of MediaFolder.Search")

Assisted-by: Claude:claude-opus-5 valgrind
---
 profiles/audio/player.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 7c5ea5b62..4c94dcab4 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -826,7 +826,7 @@ static DBusMessage *media_folder_search(DBusConnection *conn, DBusMessage *msg,
 		return btd_error_not_supported(msg);
 
 	if (mp->msg != NULL)
-		return btd_error_failed(msg, strerror(EINVAL));
+		return btd_error_failed(msg, strerror(EBUSY));
 
 	err = cb->cbs->search(mp, string, cb->user_data);
 	if (err < 0)
-- 
2.54.0 (Apple Git-157)


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH BlueZ 5/5] unit/test-media-player: Add media player tests
  2026-08-21 19:34 [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request George Kiagiadakis
                   ` (3 preceding siblings ...)
  2026-08-21 19:34 ` [PATCH BlueZ 4/5] player: Report EBUSY from a busy Search() George Kiagiadakis
@ 2026-08-21 19:34 ` George Kiagiadakis
  2026-08-24 20:40 ` [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request patchwork-bot+bluetooth
  5 siblings, 0 replies; 8+ messages in thread
From: George Kiagiadakis @ 2026-08-21 19:34 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: George Kiagiadakis

Cover the D-Bus surface of profiles/audio/player.c that the AVRCP
controller drives, using a private session bus.

Nine tests:

  play_item_without_scope        Play() on a /NowPlaying item of a
                                 player that never set a scope
  play_item_with_scope           the browsable case still works
  play_item_busy                 a second overlapping Play() is refused
  play_item_busy_with_list_items Play() during a pending ListItems() is
                                 refused, pinning the mutual exclusion
                                 between the two
  list_items_scope_change        a pending ListItems() is still answered
                                 when the scope moves meanwhile
  no_folder_without_scope        MediaFolder1 is not registered without
                                 a scope, which is why MediaItem1 was
                                 the only entry point able to observe
                                 an unset one
  play_item_destroy_pending      destroying a player answers whatever
                                 request is still in flight
  total_items_scope_change       the count reported by the player is
                                 applied when the scope moves
  search_busy                    a busy Search() reports EBUSY

Against the tree before this series play_item_without_scope,
play_item_busy and play_item_destroy_pending crash, all three because
they play a /NowPlaying item on a player with no scope,
list_items_scope_change times out with NoReply, total_items_scope_change
reads a stale count and search_busy reports the wrong error. The
remaining three pass there as well and guard against regressions.

Assisted-by: Claude:claude-opus-5 valgrind
---
 .gitignore               |   1 +
 Makefile.am              |  13 +
 unit/test-media-player.c | 838 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 852 insertions(+)
 create mode 100644 unit/test-media-player.c

diff --git a/.gitignore b/.gitignore
index c5efe8536..8485f3f46 100644
--- a/.gitignore
+++ b/.gitignore
@@ -105,6 +105,7 @@ unit/test-uuid
 unit/test-crc
 unit/test-textfile
 unit/test-gdbus-client
+unit/test-media-player
 unit/test-sdp
 unit/test-lib
 unit/test-mgmt
diff --git a/Makefile.am b/Makefile.am
index 2754e1b7f..ab7457252 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -687,6 +687,19 @@ unit_test_gdbus_client_SOURCES = unit/test-gdbus-client.c
 unit_test_gdbus_client_LDADD = gdbus/libgdbus-internal.la \
 				src/libshared-glib.la $(GLIB_LIBS) $(DBUS_LIBS)
 
+unit_tests += unit/test-media-player
+
+unit_test_media_player_SOURCES = unit/test-media-player.c \
+				profiles/audio/player.h \
+				profiles/audio/player.c \
+				src/log.h src/log.c \
+				src/error.h src/error.c \
+				src/dbus-common.h src/dbus-common.c
+unit_test_media_player_LDADD = gdbus/libgdbus-internal.la \
+				src/libshared-glib.la \
+				lib/libbluetooth-internal.la \
+				$(GLIB_LIBS) $(DBUS_LIBS)
+
 if OBEX
 unit_tests += unit/test-gobex-header unit/test-gobex-packet unit/test-gobex \
 			unit/test-gobex-transfer unit/test-gobex-apparam
diff --git a/unit/test-media-player.c b/unit/test-media-player.c
new file mode 100644
index 000000000..ef2d5dff3
--- /dev/null
+++ b/unit/test-media-player.c
@@ -0,0 +1,838 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2026  Collabora Ltd.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdbool.h>
+#include <stdint.h>
+#include <errno.h>
+#include <inttypes.h>
+#include <string.h>
+
+#include <glib.h>
+#include <dbus/dbus.h>
+
+#include "gdbus/gdbus.h"
+
+#include "src/shared/tester.h"
+#include "src/dbus-common.h"
+#include "profiles/audio/player.h"
+
+#define ERROR_INTERFACE	"org.bluez.Error"
+
+#define SERVICE_NAME	"org.bluez.unit.test-media-player"
+#define DEVICE_PATH	"/org/bluez/unit/dev_00_00_00_00_00_00"
+#define PLAYER_PATH	DEVICE_PATH "/avrcp/player0"
+#define NOWPLAYING_PATH	PLAYER_PATH "/NowPlaying/item2"
+#define FILESYSTEM_PATH	PLAYER_PATH "/Filesystem/item2"
+
+#define ITEM_UID	2
+#define TOTAL_ITEMS	42
+
+struct context {
+	DBusConnection *dbus_conn;
+	struct media_player *mp;
+	DBusPendingCall *pending;
+	DBusPendingCall *pending2;
+	bool defer_completion;
+	unsigned int play_item_calls;
+	unsigned int list_items_calls;
+	unsigned int expected_play_calls;
+	char *play_item_name;
+	uint64_t play_item_uid;
+	const char *expected_name;
+	bool change_scope_on_list;
+	bool destroy_on_play;
+	bool check_total_items;
+	unsigned int total_items_calls;
+};
+
+static struct context *context;
+
+static gboolean complete_play_item(gpointer user_data)
+{
+	if (context == NULL)
+		return FALSE;
+
+	/*
+	 * Completion has to be deferred until media_item_play() has
+	 * returned, since the pending message is only stored once the
+	 * play_item callback has succeeded.
+	 */
+	media_player_play_item_complete(context->mp, 0);
+
+	return FALSE;
+}
+
+static gboolean destroy_player(gpointer user_data)
+{
+	if (context == NULL || context->mp == NULL)
+		return FALSE;
+
+	/*
+	 * Destroyed from the main loop rather than from the callback, so
+	 * that the pending message has already been stored.
+	 */
+	media_player_destroy(context->mp);
+	context->mp = NULL;
+
+	return FALSE;
+}
+
+static int test_play_item(struct media_player *mp, const char *name,
+					uint64_t uid, void *user_data)
+{
+	struct context *ctx = user_data;
+
+	ctx->play_item_calls++;
+	g_free(ctx->play_item_name);
+	ctx->play_item_name = g_strdup(name);
+	ctx->play_item_uid = uid;
+
+	if (ctx->destroy_on_play)
+		g_idle_add(destroy_player, NULL);
+	else if (!ctx->defer_completion)
+		g_idle_add(complete_play_item, NULL);
+
+	return 0;
+}
+
+static gboolean complete_list_items(gpointer user_data)
+{
+	if (context == NULL)
+		return FALSE;
+
+	/*
+	 * A browsed player can be re-addressed at any time, which makes
+	 * avrcp call media_player_set_folder() and move the scope while
+	 * the ListItems request is still pending.
+	 */
+	if (context->change_scope_on_list)
+		media_player_set_folder(context->mp, "/Other", 1);
+
+	media_player_list_complete(context->mp, NULL, 0);
+
+	return FALSE;
+}
+
+static int test_list_items(struct media_player *mp, const char *name,
+			uint32_t start, uint32_t end, void *user_data)
+{
+	struct context *ctx = user_data;
+
+	ctx->list_items_calls++;
+
+	if (!ctx->defer_completion)
+		g_idle_add(complete_list_items, NULL);
+
+	return 0;
+}
+
+static void call_get_number_of_items(DBusPendingCall **pending,
+					DBusPendingCallNotifyFunction notify);
+static void number_of_items_reply(DBusPendingCall *call, void *user_data);
+
+static gboolean complete_total_items(gpointer user_data)
+{
+	if (context == NULL)
+		return FALSE;
+
+	media_player_total_items_complete(context->mp, TOTAL_ITEMS);
+
+	call_get_number_of_items(&context->pending2, number_of_items_reply);
+
+	return FALSE;
+}
+
+static int test_search(struct media_player *mp, const char *string,
+							void *user_data)
+{
+	return 0;
+}
+
+static int test_total_items(struct media_player *mp, const char *name,
+							void *user_data)
+{
+	struct context *ctx = user_data;
+
+	ctx->total_items_calls++;
+
+	if (ctx->check_total_items)
+		g_idle_add(complete_total_items, NULL);
+
+	return 0;
+}
+
+static const struct media_player_callback test_callbacks = {
+	.play_item = test_play_item,
+	.list_items = test_list_items,
+	.total_items = test_total_items,
+	.search = test_search,
+};
+
+static struct context *create_context(void)
+{
+	struct context *ctx = g_new0(struct context, 1);
+	DBusError err;
+
+	dbus_error_init(&err);
+
+	ctx->dbus_conn = g_dbus_setup_private(DBUS_BUS_SESSION, SERVICE_NAME,
+									&err);
+	if (ctx->dbus_conn == NULL) {
+		if (dbus_error_is_set(&err)) {
+			tester_debug("D-Bus setup failed: %s", err.message);
+			dbus_error_free(&err);
+		}
+
+		g_free(ctx);
+		tester_test_abort();
+		return NULL;
+	}
+
+	/* Avoid D-Bus library calling _exit() before next test finishes. */
+	dbus_connection_set_exit_on_disconnect(ctx->dbus_conn, FALSE);
+
+	g_dbus_attach_object_manager(ctx->dbus_conn);
+
+	set_dbus_connection(ctx->dbus_conn);
+
+	ctx->mp = media_player_controller_create(DEVICE_PATH, "avrcp", 0);
+	g_assert(ctx->mp != NULL);
+
+	media_player_set_callbacks(ctx->mp, &test_callbacks, ctx);
+
+	return ctx;
+}
+
+static void destroy_context(void)
+{
+	if (context == NULL)
+		return;
+
+	if (context->pending) {
+		dbus_pending_call_cancel(context->pending);
+		dbus_pending_call_unref(context->pending);
+	}
+
+	if (context->pending2) {
+		dbus_pending_call_cancel(context->pending2);
+		dbus_pending_call_unref(context->pending2);
+	}
+
+	if (context->mp)
+		media_player_destroy(context->mp);
+
+	set_dbus_connection(NULL);
+
+	g_dbus_detach_object_manager(context->dbus_conn);
+
+	dbus_connection_flush(context->dbus_conn);
+	dbus_connection_close(context->dbus_conn);
+	dbus_connection_unref(context->dbus_conn);
+
+	g_free(context->play_item_name);
+	g_free(context);
+	context = NULL;
+}
+
+static void play_reply(DBusPendingCall *call, void *user_data)
+{
+	struct context *ctx = user_data;
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+
+	if (reply == NULL) {
+		tester_warn("Play() got no reply");
+		tester_test_failed();
+		return;
+	}
+
+	if (dbus_message_get_type(reply) == DBUS_MESSAGE_TYPE_ERROR) {
+		tester_warn("Play() returned error: %s",
+					dbus_message_get_error_name(reply));
+		goto failed;
+	}
+
+	if (ctx->play_item_calls != 1) {
+		tester_warn("play_item called %u times, expected 1",
+						ctx->play_item_calls);
+		goto failed;
+	}
+
+	if (g_strcmp0(ctx->play_item_name, ctx->expected_name)) {
+		tester_warn("play_item name is '%s', expected '%s'",
+				ctx->play_item_name, ctx->expected_name);
+		goto failed;
+	}
+
+	if (ctx->play_item_uid != ITEM_UID) {
+		tester_warn("play_item uid is %" PRIu64 ", expected %u",
+					ctx->play_item_uid, ITEM_UID);
+		goto failed;
+	}
+
+	dbus_message_unref(reply);
+	tester_test_passed();
+	return;
+
+failed:
+	dbus_message_unref(reply);
+	tester_test_failed();
+}
+
+/* The first Play() of the busy test is never completed on purpose. */
+static void ignore_reply(DBusPendingCall *call, void *user_data)
+{
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+
+	if (reply)
+		dbus_message_unref(reply);
+}
+
+/* The second, overlapping Play() is the one under test here. */
+static void busy_reply(DBusPendingCall *call, void *user_data)
+{
+	struct context *ctx = user_data;
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+
+	if (reply == NULL) {
+		tester_warn("Play() got no reply");
+		tester_test_failed();
+		return;
+	}
+
+	if (dbus_message_get_type(reply) != DBUS_MESSAGE_TYPE_ERROR) {
+		tester_warn("overlapping Play() succeeded, expected an error");
+		goto failed;
+	}
+
+	if (ctx->play_item_calls != ctx->expected_play_calls) {
+		tester_warn("play_item called %u times, expected %u",
+				ctx->play_item_calls, ctx->expected_play_calls);
+		goto failed;
+	}
+
+	dbus_message_unref(reply);
+	tester_test_passed();
+	return;
+
+failed:
+	dbus_message_unref(reply);
+	tester_test_failed();
+}
+
+/* ListItems must always be answered, even if the scope moved meanwhile. */
+static void list_reply(DBusPendingCall *call, void *user_data)
+{
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+
+	if (reply == NULL) {
+		tester_warn("ListItems() got no reply");
+		tester_test_failed();
+		return;
+	}
+
+	if (dbus_message_get_type(reply) == DBUS_MESSAGE_TYPE_ERROR) {
+		tester_warn("ListItems() returned error: %s",
+					dbus_message_get_error_name(reply));
+		dbus_message_unref(reply);
+		tester_test_failed();
+		return;
+	}
+
+	dbus_message_unref(reply);
+	tester_test_passed();
+}
+
+/* Without a scope org.bluez.MediaFolder1 must not be registered at all. */
+static void no_folder_reply(DBusPendingCall *call, void *user_data)
+{
+	struct context *ctx = user_data;
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+
+	if (reply == NULL) {
+		tester_warn("ListItems() got no reply");
+		tester_test_failed();
+		return;
+	}
+
+	if (dbus_message_get_type(reply) != DBUS_MESSAGE_TYPE_ERROR) {
+		tester_warn("ListItems() succeeded without a scope");
+		goto failed;
+	}
+
+	if (ctx->list_items_calls != 0) {
+		tester_warn("list_items called %u times, expected 0",
+						ctx->list_items_calls);
+		goto failed;
+	}
+
+	dbus_message_unref(reply);
+	tester_test_passed();
+	return;
+
+failed:
+	dbus_message_unref(reply);
+	tester_test_failed();
+}
+
+/* Destroying the player must answer whatever request is in flight. */
+static void destroyed_reply(DBusPendingCall *call, void *user_data)
+{
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+	const char *name;
+
+	if (reply == NULL) {
+		tester_warn("Play() got no reply");
+		tester_test_failed();
+		return;
+	}
+
+	if (dbus_message_get_type(reply) != DBUS_MESSAGE_TYPE_ERROR) {
+		tester_warn("Play() succeeded, expected an error");
+		goto failed;
+	}
+
+	name = dbus_message_get_error_name(reply);
+	if (g_strcmp0(name, ERROR_INTERFACE ".Failed")) {
+		tester_warn("Play() failed with '%s', expected '%s'", name,
+						ERROR_INTERFACE ".Failed");
+		goto failed;
+	}
+
+	dbus_message_unref(reply);
+	tester_test_passed();
+	return;
+
+failed:
+	dbus_message_unref(reply);
+	tester_test_failed();
+}
+
+/* NumberOfItems has to pick up the count reported by the player. */
+static void number_of_items_reply(DBusPendingCall *call, void *user_data)
+{
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+	DBusMessageIter iter, var;
+	dbus_uint32_t items;
+
+	if (reply == NULL) {
+		tester_warn("Get(NumberOfItems) got no reply");
+		tester_test_failed();
+		return;
+	}
+
+	if (dbus_message_get_type(reply) == DBUS_MESSAGE_TYPE_ERROR) {
+		tester_warn("Get(NumberOfItems) returned error: %s",
+					dbus_message_get_error_name(reply));
+		goto failed;
+	}
+
+	if (!dbus_message_iter_init(reply, &iter) ||
+			dbus_message_iter_get_arg_type(&iter) !=
+							DBUS_TYPE_VARIANT) {
+		tester_warn("Get(NumberOfItems) reply is malformed");
+		goto failed;
+	}
+
+	dbus_message_iter_recurse(&iter, &var);
+	if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_UINT32) {
+		tester_warn("NumberOfItems is not a uint32");
+		goto failed;
+	}
+
+	dbus_message_iter_get_basic(&var, &items);
+
+	if (items != TOTAL_ITEMS) {
+		tester_warn("NumberOfItems is %u, expected %u", items,
+								TOTAL_ITEMS);
+		goto failed;
+	}
+
+	dbus_message_unref(reply);
+	tester_test_passed();
+	return;
+
+failed:
+	dbus_message_unref(reply);
+	tester_test_failed();
+}
+
+static void call_get_number_of_items(DBusPendingCall **pending,
+					DBusPendingCallNotifyFunction notify)
+{
+	DBusMessage *msg;
+	const char *iface = "org.bluez.MediaFolder1";
+	const char *prop = "NumberOfItems";
+
+	msg = dbus_message_new_method_call(SERVICE_NAME, PLAYER_PATH,
+				"org.freedesktop.DBus.Properties", "Get");
+	g_assert(msg != NULL);
+
+	g_assert(dbus_message_append_args(msg, DBUS_TYPE_STRING, &iface,
+						DBUS_TYPE_STRING, &prop,
+						DBUS_TYPE_INVALID));
+
+	g_assert(dbus_connection_send_with_reply(context->dbus_conn, msg,
+							pending, 2000));
+	g_assert(*pending != NULL);
+
+	g_assert(dbus_pending_call_set_notify(*pending, notify, context, NULL));
+
+	dbus_message_unref(msg);
+}
+
+/* A busy Search() has to report the same error as its siblings. */
+static void search_busy_reply(DBusPendingCall *call, void *user_data)
+{
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+	const char *desc;
+
+	if (reply == NULL) {
+		tester_warn("Search() got no reply");
+		tester_test_failed();
+		return;
+	}
+
+	if (dbus_message_get_type(reply) != DBUS_MESSAGE_TYPE_ERROR) {
+		tester_warn("Search() succeeded, expected an error");
+		goto failed;
+	}
+
+	if (!dbus_message_get_args(reply, NULL, DBUS_TYPE_STRING, &desc,
+							DBUS_TYPE_INVALID)) {
+		tester_warn("Search() error carries no description");
+		goto failed;
+	}
+
+	if (g_strcmp0(desc, strerror(EBUSY))) {
+		tester_warn("Search() failed with '%s', expected '%s'", desc,
+							strerror(EBUSY));
+		goto failed;
+	}
+
+	dbus_message_unref(reply);
+	tester_test_passed();
+	return;
+
+failed:
+	dbus_message_unref(reply);
+	tester_test_failed();
+}
+
+static void call_search(DBusPendingCall **pending,
+					DBusPendingCallNotifyFunction notify)
+{
+	DBusMessage *msg;
+	DBusMessageIter iter, dict;
+	const char *string = "needle";
+
+	msg = dbus_message_new_method_call(SERVICE_NAME, PLAYER_PATH,
+					"org.bluez.MediaFolder1", "Search");
+	g_assert(msg != NULL);
+
+	dbus_message_iter_init_append(msg, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &string);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "{sv}", &dict);
+	dbus_message_iter_close_container(&iter, &dict);
+
+	g_assert(dbus_connection_send_with_reply(context->dbus_conn, msg,
+							pending, 2000));
+	g_assert(*pending != NULL);
+
+	g_assert(dbus_pending_call_set_notify(*pending, notify, context, NULL));
+
+	dbus_message_unref(msg);
+}
+
+static void call_list_items(DBusPendingCall **pending,
+					DBusPendingCallNotifyFunction notify)
+{
+	DBusMessage *msg;
+	DBusMessageIter iter, dict;
+
+	msg = dbus_message_new_method_call(SERVICE_NAME, PLAYER_PATH,
+					"org.bluez.MediaFolder1", "ListItems");
+	g_assert(msg != NULL);
+
+	dbus_message_iter_init_append(msg, &iter);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "{sv}", &dict);
+	dbus_message_iter_close_container(&iter, &dict);
+
+	g_assert(dbus_connection_send_with_reply(context->dbus_conn, msg,
+							pending, 2000));
+	g_assert(*pending != NULL);
+
+	g_assert(dbus_pending_call_set_notify(*pending, notify, context, NULL));
+
+	dbus_message_unref(msg);
+}
+
+static void call_play(const char *path, DBusPendingCall **pending,
+					DBusPendingCallNotifyFunction notify)
+{
+	DBusMessage *msg;
+
+	msg = dbus_message_new_method_call(SERVICE_NAME, path,
+						"org.bluez.MediaItem1", "Play");
+	g_assert(msg != NULL);
+
+	g_assert(dbus_connection_send_with_reply(context->dbus_conn, msg,
+							pending, 2000));
+	g_assert(*pending != NULL);
+
+	g_assert(dbus_pending_call_set_notify(*pending, notify, context, NULL));
+
+	dbus_message_unref(msg);
+}
+
+static struct media_item *create_nowplaying_item(void)
+{
+	g_assert(media_player_create_folder(context->mp, "/NowPlaying",
+					PLAYER_FOLDER_TYPE_MIXED, 0) != NULL);
+	media_player_set_playlist(context->mp, "/NowPlaying");
+
+	return media_player_set_playlist_item(context->mp, ITEM_UID);
+}
+
+/*
+ * A player advertising the NowPlaying feature bit but not the Browsing
+ * feature bit gets a /NowPlaying folder holding playable items, while the
+ * player scope stays unset because SetBrowsedPlayer is never issued.
+ *
+ * Playing such an item must not dereference the unset scope.
+ */
+static void test_play_item_without_scope(const void *data)
+{
+	context = create_context();
+	if (context == NULL)
+		return;
+
+	context->expected_name = NOWPLAYING_PATH;
+
+	g_assert(create_nowplaying_item() != NULL);
+
+	call_play(NOWPLAYING_PATH, &context->pending, play_reply);
+}
+
+static struct media_item *create_filesystem_item(void)
+{
+	struct media_item *item;
+
+	g_assert(media_player_create_folder(context->mp, "/Filesystem",
+					PLAYER_FOLDER_TYPE_MIXED, 0) != NULL);
+	media_player_set_folder(context->mp, "/Filesystem", 1);
+
+	item = media_player_create_item(context->mp, "track",
+					PLAYER_ITEM_TYPE_AUDIO, ITEM_UID);
+	g_assert(item != NULL);
+	media_item_set_playable(item, true);
+
+	return item;
+}
+
+/* A browsable player sets a scope, which must keep working. */
+static void test_play_item_with_scope(const void *data)
+{
+	context = create_context();
+	if (context == NULL)
+		return;
+
+	context->expected_name = FILESYSTEM_PATH;
+
+	g_assert(create_filesystem_item() != NULL);
+
+	call_play(FILESYSTEM_PATH, &context->pending, play_reply);
+}
+
+/* A Play() issued while another one is still pending must be rejected. */
+static void test_play_item_busy(const void *data)
+{
+	context = create_context();
+	if (context == NULL)
+		return;
+
+	context->expected_name = NOWPLAYING_PATH;
+	context->defer_completion = true;
+	context->expected_play_calls = 1;
+
+	g_assert(create_nowplaying_item() != NULL);
+
+	call_play(NOWPLAYING_PATH, &context->pending, ignore_reply);
+	call_play(NOWPLAYING_PATH, &context->pending2, busy_reply);
+}
+
+/*
+ * ListItems() and Play() share one pending-request slot per player, so a
+ * Play() issued while a ListItems() is still outstanding must be rejected.
+ */
+static void test_play_item_busy_with_list_items(const void *data)
+{
+	context = create_context();
+	if (context == NULL)
+		return;
+
+	context->defer_completion = true;
+	context->expected_play_calls = 0;
+
+	g_assert(create_filesystem_item() != NULL);
+
+	call_list_items(&context->pending, ignore_reply);
+	call_play(FILESYSTEM_PATH, &context->pending2, busy_reply);
+}
+
+/*
+ * The scope can move while a request is pending, which is what avrcp does
+ * on SetBrowsedPlayer. The outstanding ListItems() must still be answered.
+ */
+static void test_list_items_scope_change(const void *data)
+{
+	context = create_context();
+	if (context == NULL)
+		return;
+
+	context->change_scope_on_list = true;
+
+	g_assert(media_player_create_folder(context->mp, "/Filesystem",
+					PLAYER_FOLDER_TYPE_MIXED, 0) != NULL);
+	g_assert(media_player_create_folder(context->mp, "/Other",
+					PLAYER_FOLDER_TYPE_MIXED, 0) != NULL);
+	media_player_set_folder(context->mp, "/Filesystem", 1);
+
+	call_list_items(&context->pending, list_reply);
+}
+
+/*
+ * Without a scope MediaFolder1 is not registered at all, which is why
+ * MediaItem1.Play() was the only entry point reachable with an unset scope.
+ */
+static void test_no_folder_without_scope(const void *data)
+{
+	context = create_context();
+	if (context == NULL)
+		return;
+
+	g_assert(create_nowplaying_item() != NULL);
+
+	call_list_items(&context->pending, no_folder_reply);
+}
+
+/*
+ * Destroying a player while a request is still in flight must answer it,
+ * rather than leave the caller waiting for the D-Bus timeout.
+ */
+static void test_play_item_destroy_pending(const void *data)
+{
+	context = create_context();
+	if (context == NULL)
+		return;
+
+	context->defer_completion = true;
+	context->destroy_on_play = true;
+
+	g_assert(create_nowplaying_item() != NULL);
+
+	call_play(NOWPLAYING_PATH, &context->pending, destroyed_reply);
+}
+
+/*
+ * A player reporting the total number of items has to see it applied,
+ * including when the scope moves without any D-Bus request in flight,
+ * which is what avrcp does on SetBrowsedPlayer.
+ */
+static void test_total_items_scope_change(const void *data)
+{
+	context = create_context();
+	if (context == NULL)
+		return;
+
+	context->check_total_items = true;
+
+	g_assert(media_player_create_folder(context->mp, "/Filesystem",
+					PLAYER_FOLDER_TYPE_MIXED, 0) != NULL);
+	g_assert(media_player_create_folder(context->mp, "/Other",
+					PLAYER_FOLDER_TYPE_MIXED, 0) != NULL);
+
+	media_player_set_folder(context->mp, "/Filesystem", 0);
+	media_player_set_folder(context->mp, "/Other", 0);
+
+	g_assert(context->total_items_calls == 1);
+}
+
+/*
+ * Search() shares the pending request slot with the other browsing
+ * requests, so a busy Search() must report EBUSY like they do.
+ */
+static void test_search_busy(const void *data)
+{
+	context = create_context();
+	if (context == NULL)
+		return;
+
+	context->defer_completion = true;
+
+	g_assert(media_player_create_folder(context->mp, "/Filesystem",
+					PLAYER_FOLDER_TYPE_MIXED, 0) != NULL);
+	media_player_set_folder(context->mp, "/Filesystem", 1);
+	media_player_set_searchable(context->mp, true);
+
+	call_list_items(&context->pending, ignore_reply);
+	call_search(&context->pending2, search_busy_reply);
+}
+
+static void test_teardown(const void *data)
+{
+	destroy_context();
+
+	tester_teardown_complete();
+}
+
+int main(int argc, char *argv[])
+{
+	tester_init(&argc, &argv);
+
+	tester_add("/media_player/play_item_without_scope", NULL, NULL,
+					test_play_item_without_scope,
+					test_teardown);
+
+	tester_add("/media_player/play_item_with_scope", NULL, NULL,
+					test_play_item_with_scope,
+					test_teardown);
+
+	tester_add("/media_player/play_item_busy", NULL, NULL,
+					test_play_item_busy,
+					test_teardown);
+
+	tester_add("/media_player/play_item_busy_with_list_items", NULL, NULL,
+					test_play_item_busy_with_list_items,
+					test_teardown);
+
+	tester_add("/media_player/list_items_scope_change", NULL, NULL,
+					test_list_items_scope_change,
+					test_teardown);
+
+	tester_add("/media_player/no_folder_without_scope", NULL, NULL,
+					test_no_folder_without_scope,
+					test_teardown);
+
+	tester_add("/media_player/play_item_destroy_pending", NULL, NULL,
+					test_play_item_destroy_pending,
+					test_teardown);
+
+	tester_add("/media_player/total_items_scope_change", NULL, NULL,
+					test_total_items_scope_change,
+					test_teardown);
+
+	tester_add("/media_player/search_busy", NULL, NULL,
+					test_search_busy, test_teardown);
+
+	return tester_run();
+}
-- 
2.54.0 (Apple Git-157)


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: player: Fix crash and related defects around the pending request
  2026-08-21 19:34 ` [PATCH BlueZ 1/5] player: Fix crash on MediaItem1.Play() without a browsing scope George Kiagiadakis
@ 2026-08-21 20:38   ` bluez.test.bot
  0 siblings, 0 replies; 8+ messages in thread
From: bluez.test.bot @ 2026-08-21 20:38 UTC (permalink / raw)
  To: linux-bluetooth, george.kiagiadakis

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

---Test result---

Test Summary:
CheckPatch                    FAIL      2.91 seconds
GitLint                       FAIL      1.60 seconds
BuildEll                      PASS      20.28 seconds
BluezMake                     PASS      549.74 seconds
MakeCheck                     PASS      19.22 seconds
MakeDistcheck                 PASS      154.17 seconds
CheckValgrind                 PASS      224.64 seconds
CheckSmatch                   PASS      297.60 seconds
bluezmakeextell               PASS      96.20 seconds
IncrementalBuild              PASS      586.08 seconds
ScanBuild                     PASS      892.57 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,1/5] player: Fix crash on MediaItem1.Play() without a browsing scope
WARNING:BAD_SIGN_OFF: Non-standard signature: Assisted-by:
#112: 
Assisted-by: Claude:claude-opus-5 valgrind

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Claude:claude-opus-5 valgrind'
#112: 
Assisted-by: Claude:claude-opus-5 valgrind

/github/workspace/src/patch/14762403.patch total: 1 errors, 1 warnings, 246 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14762403.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


[BlueZ,2/5] player: Answer pending request when the player is destroyed
WARNING:BAD_SIGN_OFF: Non-standard signature: Assisted-by:
#84: 
Assisted-by: Claude:claude-opus-5 valgrind

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Claude:claude-opus-5 valgrind'
#84: 
Assisted-by: Claude:claude-opus-5 valgrind

/github/workspace/src/patch/14762405.patch total: 1 errors, 1 warnings, 12 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14762405.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


[BlueZ,3/5] player: Fix NumberOfItems never being updated on SetBrowsedPlayer
WARNING:BAD_SIGN_OFF: Non-standard signature: Assisted-by:
#103: 
Assisted-by: Claude:claude-opus-5 valgrind

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Claude:claude-opus-5 valgrind'
#103: 
Assisted-by: Claude:claude-opus-5 valgrind

/github/workspace/src/patch/14762404.patch total: 1 errors, 1 warnings, 8 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14762404.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


[BlueZ,4/5] player: Report EBUSY from a busy Search()
WARNING:BAD_SIGN_OFF: Non-standard signature: Assisted-by:
#81: 
Assisted-by: Claude:claude-opus-5 valgrind

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Claude:claude-opus-5 valgrind'
#81: 
Assisted-by: Claude:claude-opus-5 valgrind

/github/workspace/src/patch/14762406.patch total: 1 errors, 1 warnings, 8 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14762406.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


[BlueZ,5/5] unit/test-media-player: Add media player tests
WARNING:BAD_SIGN_OFF: Non-standard signature: Assisted-by:
#101: 
Assisted-by: Claude:claude-opus-5 valgrind

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Claude:claude-opus-5 valgrind'
#101: 
Assisted-by: Claude:claude-opus-5 valgrind

/github/workspace/src/patch/14762407.patch total: 1 errors, 1 warnings, 864 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14762407.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,1/5] player: Fix crash on MediaItem1.Play() without a browsing scope

5: B3 Line contains hard tab characters (\t): "	struct media_folder *folder = mp->scope;"
6: B3 Line contains hard tab characters (\t): "	..."
7: B3 Line contains hard tab characters (\t): "	if (folder->msg)"
[BlueZ,3/5] player: Fix NumberOfItems never being updated on SetBrowsedPlayer

6: B3 Line contains hard tab characters (\t): "	if (folder == NULL || folder->msg == NULL)"
7: B3 Line contains hard tab characters (\t): "		return;"


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

---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request
  2026-08-21 19:34 [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request George Kiagiadakis
                   ` (4 preceding siblings ...)
  2026-08-21 19:34 ` [PATCH BlueZ 5/5] unit/test-media-player: Add media player tests George Kiagiadakis
@ 2026-08-24 20:40 ` patchwork-bot+bluetooth
  5 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+bluetooth @ 2026-08-24 20:40 UTC (permalink / raw)
  To: George Kiagiadakis; +Cc: linux-bluetooth

Hello:

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

On Fri, 21 Aug 2026 22:34:44 +0300 you wrote:
> A player that advertises the AVRCP NowPlaying feature bit but not the
> Browsing bit exports playable MediaItem1 objects while the player scope
> is still unset, and calling org.bluez.MediaItem1.Play() on one of them
> crashes bluetoothd with a NULL dereference at offset 0x28.
> 
> media_item_play() dereferences mp->scope, which is only ever set from
> SetBrowsedPlayer or ChangeFolder, and both are reached only when the
> player advertises Browsing (features[7] & 0x08). The /NowPlaying folder
> and its playable items are gated on a different bit (features[8] &
> 0x02), so the two can disagree. msg sits at offset 40 in struct
> media_folder on LP64, which is the reported fault address.
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/5] player: Fix crash on MediaItem1.Play() without a browsing scope
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a8d22214940d
  - [BlueZ,2/5] player: Answer pending request when the player is destroyed
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ede23fb50e41
  - [BlueZ,3/5] player: Fix NumberOfItems never being updated on SetBrowsedPlayer
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=3215010456f1
  - [BlueZ,4/5] player: Report EBUSY from a busy Search()
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a93047cd044f
  - [BlueZ,5/5] unit/test-media-player: Add media player tests
    (no matching commit)

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-24 20:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21 19:34 [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request George Kiagiadakis
2026-08-21 19:34 ` [PATCH BlueZ 1/5] player: Fix crash on MediaItem1.Play() without a browsing scope George Kiagiadakis
2026-08-21 20:38   ` player: Fix crash and related defects around the pending request bluez.test.bot
2026-08-21 19:34 ` [PATCH BlueZ 2/5] player: Answer pending request when the player is destroyed George Kiagiadakis
2026-08-21 19:34 ` [PATCH BlueZ 3/5] player: Fix NumberOfItems never being updated on SetBrowsedPlayer George Kiagiadakis
2026-08-21 19:34 ` [PATCH BlueZ 4/5] player: Report EBUSY from a busy Search() George Kiagiadakis
2026-08-21 19:34 ` [PATCH BlueZ 5/5] unit/test-media-player: Add media player tests George Kiagiadakis
2026-08-24 20:40 ` [PATCH BlueZ 0/5] player: Fix crash and related defects around the pending request patchwork-bot+bluetooth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.