[BlueZ 1/2] mpris-proxy: Fix possible crash

[BlueZ 1/2] mpris-proxy: Fix possible crash

[Bastien Nocera]

find_player_by_obex() doesn't check whether session->obex is a valid
pointer before dereferecing it, but all code paths that assign it use
create_obex_session() to assign it, a function that can fail.

Check whether session->obex is null before dereferencing it.

 #0 find_player_by_obex at tools/mpris-proxy.c:2819
 #1 obex_property_changed at tools/mpris-proxy.c:2929
 #2 add_property at gdbus/client.c:373
 #3 update_properties at gdbus/client.c:399
 #5 properties_changed at gdbus/client.c:537
 #6 signal_filter at gdbus/watch.c:416
 #7 message_filter at gdbus/watch.c:566
 #10 message_dispatch at gdbus/mainloop.c:59
 #13 g_main_context_dispatch_unlocked at ../glib/gmain.c:4451
 #14 g_main_context_iterate_unlocked at ../glib/gmain.c:4516

Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2466640
---
 tools/mpris-proxy.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/mpris-proxy.c b/tools/mpris-proxy.c
index 1d7a421e9278..360b8ab34469 100644
--- a/tools/mpris-proxy.c
+++ b/tools/mpris-proxy.c
@@ -2816,8 +2816,12 @@ static struct player *find_player_by_obex(const char *path)
 	for (l = players; l; l = l->next) {
 		struct player *player = l->data;
 		struct obex_session *session = player->obex;
-		const char *obex_path = g_dbus_proxy_get_path(session->obex);
+		const char *obex_path = NULL;
 
+		if (session == NULL)
+			continue;
+
+		obex_path= g_dbus_proxy_get_path(session->obex);
 		if (g_str_has_prefix(path, obex_path))
 			return player;
 	}
-- 
2.54.0

[BlueZ 2/2] mpris-proxy: Avoid session->obex dereference

[Bastien Nocera]

Protect against trying to access session->obex variable that might be
null after failure to create the session object.
---
 tools/mpris-proxy.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/mpris-proxy.c b/tools/mpris-proxy.c
index 360b8ab34469..8c9b01044fe2 100644
--- a/tools/mpris-proxy.c
+++ b/tools/mpris-proxy.c
@@ -1306,7 +1306,7 @@ static int parse_track_entry(DBusMessageIter *entry, const char *key,
 		const char *handle, *path;
 		char *filename, *uri;
 
-		if (!player || !player->obex)
+		if (!player || !player->obex || !player->obex->obex)
 			return -EINVAL;
 
 		path = g_dbus_proxy_get_path(player->obex->obex);
@@ -2614,9 +2614,13 @@ static void obex_get_image(struct player *player, const char *handle)
 	DBusMessage *msg;
 	DBusMessageIter iter, array;
 	struct obex_session *obex_session = player->obex;
-	const char *path = g_dbus_proxy_get_path(obex_session->obex);
+	const char *path;
 	char *filename;
 
+	if (!player->obex)
+		return;
+
+	path = g_dbus_proxy_get_path(obex_session->obex);
 	player->filename = g_strconcat(g_get_tmp_dir(), "/",
 				path + strlen(BLUEZ_OBEX_CLIENT_PATH "/"),
 				"-", handle, NULL);
@@ -2674,7 +2678,7 @@ static void device_property_changed(GDBusProxy *proxy, const char *name,
 			player->obex = NULL;
 	}
 
-	g_dbus_proxy_unref(session->obex);
+	g_clear_pointer(&session->obex, g_dbus_proxy_unref);
 	g_dbus_proxy_unref(session->device);
 	obex_sessions = g_slist_remove(obex_sessions, session);
 	g_free(session);
-- 
2.54.0

RE: [BlueZ,1/2] mpris-proxy: Fix possible crash

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    FAIL      0.91 seconds
GitLint                       PASS      0.68 seconds
BuildEll                      PASS      20.33 seconds
BluezMake                     PASS      619.55 seconds
CheckSmatch                   PASS      324.51 seconds
bluezmakeextell               PASS      163.73 seconds
IncrementalBuild              PASS      622.47 seconds
ScanBuild                     PASS      924.11 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,1/2] mpris-proxy: Fix possible crash
ERROR:SPACING: spaces required around that '=' (ctx:VxW)
#89: FILE: tools/mpris-proxy.c:2824:
+		obex_path= g_dbus_proxy_get_path(session->obex);
 		         ^

/github/workspace/src/patch/14554528.patch total: 1 errors, 0 warnings, 13 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/14554528.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.




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

---
Regards,
Linux Bluetooth