[BlueZ,1/1] client: Fix uninitialized read in attribute handle

Commit Message

Alicia Boya Garcia Aug. 8, 2022, 8:26 a.m. UTC

When services, characteristics and descriptors were parsed from DBus
proxies the client code was calling the print code without initializing
the `handle` field, which the print functions use.

This resulted in semi-random or zero handles in all attributes when
using gatt.list-attributes in bluetoothctl, depending on compilation
flags.

This patch fixes the problem by parsing the handle from the DBus proxy
path.
---
 client/gatt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

[e-mail] Aug. 8, 2022, 9:12 a.m. UTC | #1

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=665993

---Test result---

Test Summary:
CheckPatch                    FAIL      1.93 seconds
GitLint                       PASS      0.96 seconds
Prep - Setup ELL              PASS      28.30 seconds
Build - Prep                  PASS      0.87 seconds
Build - Configure             PASS      8.67 seconds
Build - Make                  PASS      962.04 seconds
Make Check                    PASS      12.23 seconds
Make Check w/Valgrind         PASS      287.33 seconds
Make Distcheck                PASS      237.89 seconds
Build w/ext ELL - Configure   PASS      8.80 seconds
Build w/ext ELL - Make        PASS      83.00 seconds
Incremental Build w/ patches  PASS      0.00 seconds
Scan Build                    PASS      496.77 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,1/1] client: Fix uninitialized read in attribute handle
WARNING:LINE_SPACING: Missing a blank line after declarations
#104: FILE: client/gatt.c:164:
+	const char *number = path + strlen(path) - 4;
+	if (number < path)

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




---
Regards,
Linux Bluetooth

Patch

diff --git a/client/gatt.c b/client/gatt.c
index 4c1efaf75..07a024605 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -158,6 +158,15 @@  static void print_inc_service(struct service *service, const char *description)
 					service->uuid, text);
 }
 
+static uint16_t handle_from_path(const char *path)
+{
+	const char *number = path + strlen(path) - 4;
+	if (number < path)
+		return 0;
+
+	return (uint16_t) strtol(number, NULL, 16);
+}
+
 static void print_service_proxy(GDBusProxy *proxy, const char *description)
 {
 	struct service service;
@@ -179,6 +188,7 @@  static void print_service_proxy(GDBusProxy *proxy, const char *description)
 	service.path = (char *) g_dbus_proxy_get_path(proxy);
 	service.uuid = (char *) uuid;
 	service.primary = primary;
+	service.handle = handle_from_path(service.path);
 
 	print_service(&service, description);
 }
@@ -261,6 +271,7 @@  static void print_characteristic(GDBusProxy *proxy, const char *description)
 	memset(&chrc, 0, sizeof(chrc));
 	chrc.path = (char *) g_dbus_proxy_get_path(proxy);
 	chrc.uuid = (char *) uuid;
+	chrc.handle = handle_from_path(chrc.path);
 
 	print_chrc(&chrc, description);
 }
@@ -355,6 +366,7 @@  static void print_descriptor(GDBusProxy *proxy, const char *description)
 	memset(&desc, 0, sizeof(desc));
 	desc.path = (char *) g_dbus_proxy_get_path(proxy);
 	desc.uuid = (char *) uuid;
+	desc.handle = handle_from_path(desc.path);
 
 	print_desc(&desc, description);
 }