[PATCH BlueZ v1 0/2] client/cs: fix crash in reference ranging provider

[PATCH BlueZ v1 0/2] client/cs: fix crash in reference ranging provider

[Naga Bhavani Akella]

`start <dev_addr> 30` followed by `register_provider /` segfaults
bluetoothctl on the first ProcedureData signal: cs_ranging_obj_create()
exported each device's RangingProvider1 object as a direct child of
"/", the one path with real gdbus object data (its ObjectManager), and
gdbus/object.c's invalidate_parent_data() dereferences a NULL parent
in that case.

Nest the exported objects under RANGING_PROVIDER_PATH
"/org/example/ranging") instead of "/" directly.
bluetoothd's provider watch matches any descendant
of the registered root, so discovery still works.

Patch 1 is the fix, patch 2 updates the docs to match.


Naga Bhavani Akella (2):
  client: avoid registering ranging objects as direct children of "/"
  doc: note RangingProvider1 objects are nested in bluetoothctl-cs

 client/cs.c             | 28 +++++++++++++++++++++++++++-
 doc/bluetoothctl-cs.rst |  8 +++++---
 2 files changed, 32 insertions(+), 4 deletions(-)

-- 

[PATCH BlueZ v1 1/2] client: avoid registering ranging objects as direct children of "/"

[Naga Bhavani Akella]

With the default provider path "/", cs_ranging_obj_create() exported
each device's RangingProvider1 object as a direct child of root. That
triggers a NULL-dereference bug in gdbus/object.c's
invalidate_parent_data(), crashing bluetoothctl with SIGSEGV on the
first ChannelSounding1.ProcedureData signal.

Fix by nesting under RANGING_PROVIDER_PATH that is
never independently registered, so the leaf's immediate parent isn't
root. src/ranging.c's provider watch matches any descendant of the
registered root, so discovery is unaffected.
---
 client/cs.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/client/cs.c b/client/cs.c
index be546cc20..7533d053b 100644
--- a/client/cs.c
+++ b/client/cs.c
@@ -357,12 +357,23 @@ static struct cs_session *cs_find_session(GDBusProxy *proxy)
  * main()); bluetoothd's RegisterRangingProvider watches the exact path it
  * is given via GetManagedObjects()/InterfacesAdded, so any other path would
  * silently never be discovered.
+ *
+ * Exported RangingProvider1 objects are nested under RANGING_PROVIDER_PATH
+ * rather than directly under "/" (see cs_ranging_obj_create()), to avoid a
+ * gdbus/object.c bug hit when linking a brand-new direct child of a path
+ * that already has an ObjectManager attached.
  */
 
 #define RANGING_PROVIDER_INTERFACE	    "org.bluez.RangingProvider1"
 #define CS_PROCEDURE_DATA_INTERFACE	    "org.bluez.ChannelSounding1"
 #define DEFAULT_PROVIDER_PATH		    "/"
 
+/* Prefix used for exported RangingProvider1 objects when the registered
+ * provider root is "/" (see cs_ranging_obj_create()); matches the object
+ * path test/example-ranging-provider uses for the same purpose.
+ */
+#define RANGING_PROVIDER_PATH		    "/org/example/ranging"
+
 struct cs_ranging_obj {
 	char *path;		/* exported RangingProvider object path */
 	char *dev_path;		/* Device this estimate applies to */
@@ -479,8 +490,20 @@ static struct cs_ranging_obj *cs_ranging_obj_create(const char *dev_path)
 	obj = g_new0(struct cs_ranging_obj, 1);
 	obj->dev_path = g_strdup(dev_path);
 
+	/* Never export a RangingProvider1 object as a direct child of "/":
+	 * bluetoothctl attaches its ObjectManager there, so "/" already has
+	 * registered gdbus object data, and gdbus/object.c's
+	 * invalidate_parent_data() dereferences a NULL "grandparent" when
+	 * asked to link a brand-new top-level child in that state. Nesting
+	 * under RANGING_PROVIDER_PATH instead keeps the immediate parent
+	 * unregistered, so gdbus skips the ancestor walk instead of
+	 * crashing. bluetoothd's provider watch matches any descendant of
+	 * the given root (see path_has_root() in src/ranging.c), so the
+	 * extra path component doesn't affect discovery.
+	 */
 	if (!strcmp(cs_provider_path, "/"))
-		obj->path = g_strdup_printf("/%s", leaf);
+		obj->path = g_strdup_printf("%s/%s", RANGING_PROVIDER_PATH,
+						leaf);
 	else
 		obj->path = g_strdup_printf("%s/%s", cs_provider_path, leaf);
 
@@ -1537,6 +1560,9 @@ static const struct bt_shell_menu cs_menu = {
 				"\t\t\t\t\t\t[path] is the provider root object"
 				" path; default \"/\", the only path"
 				" bluetoothctl exposes an ObjectManager at.\n"
+				"\t\t\t\t\t\texported RangingProvider1 objects are"
+				" nested under /org/example/ranging rather"
+				" than directly under \"/\".\n"
 				"\t\t\t\t\t\tsee test/example-ranging-provider for a"
 				" minimal standalone provider skeleton.\n\t\t\t\t\t\tOnly one"
 				" provider may be registered per adapter at"
-- 

[PATCH BlueZ v1 2/2] doc: note RangingProvider1 objects are nested in bluetoothctl-cs

[Naga Bhavani Akella]

register_provider's exported RangingProvider1 objects live under
/org/example/ranging rather than directly under "/", to avoid a
gdbus/object.c bug when linking new top-level children of a path
that already has an ObjectManager attached. Document that alongside
the existing default-path note.
---
 doc/bluetoothctl-cs.rst | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/doc/bluetoothctl-cs.rst b/doc/bluetoothctl-cs.rst
index 08a7cafc1..c3d67fd2d 100644
--- a/doc/bluetoothctl-cs.rst
+++ b/doc/bluetoothctl-cs.rst
@@ -135,9 +135,11 @@ illustrative distance estimate (not an accurate measurement) exposed via
 
 The provider path defaults to ``/``, the only path bluetoothctl exposes an
 ``ObjectManager`` at; a different path will register but bluetoothd will not
-discover any objects under it. See **test/example-ranging-provider** for a
-minimal standalone provider skeleton, which reports a fixed placeholder
-distance and does not parse **ProcedureData** at all.
+discover any objects under it. The exported **RangingProvider1** objects
+themselves are nested under ``/org/example/ranging`` rather than directly
+under ``/``. See **test/example-ranging-provider** for a minimal standalone
+provider skeleton, which reports a fixed placeholder distance and does not
+parse **ProcedureData** at all.
 
 Only one ranging provider may be registered per adapter at a time. If a
 real ranging daemon is already registered, this command fails with
-- 

RE: client/cs: fix crash in reference ranging provider

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      0.55 seconds
GitLint                       FAIL      0.45 seconds
BuildEll                      PASS      21.03 seconds
BluezMake                     PASS      612.71 seconds
MakeCheck                     PASS      19.36 seconds
MakeDistcheck                 PASS      168.36 seconds
CheckValgrind                 PASS      238.29 seconds
CheckSmatch                   PASS      325.89 seconds
bluezmakeextell               PASS      106.60 seconds
IncrementalBuild              PASS      610.56 seconds
ScanBuild                     PASS      1034.59 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,v1,1/2] client: avoid registering ranging objects as direct children of "/"

1: T1 Title exceeds max length (82>80): "[BlueZ,v1,1/2] client: avoid registering ranging objects as direct children of "/""


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

---
Regards,
Linux Bluetooth