diff mbox series

[BlueZ] input: Fix .device_probe failing if SDP record is not found

Message ID 20231010194306.603745-1-luiz.dentz@gmail.com
State New
Headers show
Series [BlueZ] input: Fix .device_probe failing if SDP record is not found | expand

Commit Message

Luiz Augusto von Dentz Oct. 10, 2023, 7:43 p.m. UTC
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Due to changes introduced by 67a26abe53bf
("profile: Add probe_on_discover flag") profiles may get probed when
their profile UUID are discovered, rather than resolved, which means
the SDP record may not be available.

Fixes: https://github.com/bluez/bluez/issues/614
---
 profiles/input/device.c | 182 +++++++++++++++++++---------------------
 1 file changed, 84 insertions(+), 98 deletions(-)

Comments

bluez.test.bot@gmail.com Oct. 10, 2023, 9:15 p.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=791911

---Test result---

Test Summary:
CheckPatch                    PASS      0.50 seconds
GitLint                       PASS      0.25 seconds
BuildEll                      PASS      28.81 seconds
BluezMake                     PASS      913.67 seconds
MakeCheck                     PASS      12.12 seconds
MakeDistcheck                 PASS      170.71 seconds
CheckValgrind                 PASS      270.37 seconds
CheckSmatch                   WARNING   361.63 seconds
bluezmakeextell               PASS      111.37 seconds
IncrementalBuild              PASS      736.14 seconds
ScanBuild                     PASS      1116.30 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
profiles/input/device.c:165:26: warning: Variable length array is used.


---
Regards,
Linux Bluetooth
diff mbox series

Patch

diff --git a/profiles/input/device.c b/profiles/input/device.c
index e2ac6ea60352..4a50ea9921a9 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -60,7 +60,7 @@  struct input_device {
 	char			*path;
 	bdaddr_t		src;
 	bdaddr_t		dst;
-	uint32_t		handle;
+	const sdp_record_t	*rec;
 	GIOChannel		*ctrl_io;
 	GIOChannel		*intr_io;
 	guint			ctrl_watch;
@@ -754,7 +754,8 @@  static void epox_endian_quirk(unsigned char *data, int size)
 	}
 }
 
-static int create_hid_dev_name(sdp_record_t *rec, struct hidp_connadd_req *req)
+static int create_hid_dev_name(const sdp_record_t *rec,
+					struct hidp_connadd_req *req)
 {
 	char sdesc[sizeof(req->name) / 2];
 
@@ -776,7 +777,7 @@  static int create_hid_dev_name(sdp_record_t *rec, struct hidp_connadd_req *req)
 
 /* See HID profile specification v1.0, "7.11.6 HIDDescriptorList" for details
  * on the attribute format. */
-static int extract_hid_desc_data(sdp_record_t *rec,
+static int extract_hid_desc_data(const sdp_record_t *rec,
 						struct hidp_connadd_req *req)
 {
 	sdp_data_t *d;
@@ -817,36 +818,40 @@  invalid_desc:
 	return -EINVAL;
 }
 
-static int extract_hid_record(sdp_record_t *rec, struct hidp_connadd_req *req)
+static int extract_hid_record(struct input_device *idev,
+					struct hidp_connadd_req *req)
 {
 	sdp_data_t *pdlist;
 	uint8_t attr_val;
 	int err;
 
-	err = create_hid_dev_name(rec, req);
+	if (!idev->rec)
+		return -ENOENT;
+
+	err = create_hid_dev_name(idev->rec, req);
 	if (err < 0)
 		DBG("No valid Service Name or Service Description found");
 
-	pdlist = sdp_data_get(rec, SDP_ATTR_HID_PARSER_VERSION);
+	pdlist = sdp_data_get(idev->rec, SDP_ATTR_HID_PARSER_VERSION);
 	req->parser = pdlist ? pdlist->val.uint16 : 0x0100;
 
-	pdlist = sdp_data_get(rec, SDP_ATTR_HID_DEVICE_SUBCLASS);
+	pdlist = sdp_data_get(idev->rec, SDP_ATTR_HID_DEVICE_SUBCLASS);
 	req->subclass = pdlist ? pdlist->val.uint8 : 0;
 
-	pdlist = sdp_data_get(rec, SDP_ATTR_HID_COUNTRY_CODE);
+	pdlist = sdp_data_get(idev->rec, SDP_ATTR_HID_COUNTRY_CODE);
 	req->country = pdlist ? pdlist->val.uint8 : 0;
 
-	pdlist = sdp_data_get(rec, SDP_ATTR_HID_VIRTUAL_CABLE);
+	pdlist = sdp_data_get(idev->rec, SDP_ATTR_HID_VIRTUAL_CABLE);
 	attr_val = pdlist ? pdlist->val.uint8 : 0;
 	if (attr_val)
 		req->flags |= (1 << HIDP_VIRTUAL_CABLE_UNPLUG);
 
-	pdlist = sdp_data_get(rec, SDP_ATTR_HID_BOOT_DEVICE);
+	pdlist = sdp_data_get(idev->rec, SDP_ATTR_HID_BOOT_DEVICE);
 	attr_val = pdlist ? pdlist->val.uint8 : 0;
 	if (attr_val)
 		req->flags |= (1 << HIDP_BOOT_PROTOCOL_MODE);
 
-	err = extract_hid_desc_data(rec, req);
+	err = extract_hid_desc_data(idev->rec, req);
 	if (err < 0)
 		return err;
 
@@ -1035,11 +1040,6 @@  static gboolean encrypt_notify(GIOChannel *io, GIOCondition condition,
 static int hidp_add_connection(struct input_device *idev)
 {
 	struct hidp_connadd_req *req;
-	sdp_record_t *rec;
-	char src_addr[18], dst_addr[18];
-	char filename[PATH_MAX];
-	GKeyFile *key_file;
-	char handle[11], *str;
 	GError *gerr = NULL;
 	int err;
 
@@ -1049,33 +1049,7 @@  static int hidp_add_connection(struct input_device *idev)
 	req->flags     = 0;
 	req->idle_to   = idle_timeout;
 
-	ba2str(&idev->src, src_addr);
-	ba2str(&idev->dst, dst_addr);
-
-	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s", src_addr,
-								dst_addr);
-	sprintf(handle, "0x%8.8X", idev->handle);
-
-	key_file = g_key_file_new();
-	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
-		error("Unable to load key file from %s: (%s)", filename,
-								gerr->message);
-		g_clear_error(&gerr);
-	}
-	str = g_key_file_get_string(key_file, "ServiceRecords", handle, NULL);
-	g_key_file_free(key_file);
-
-	if (!str) {
-		error("Rejected connection from unknown device %s", dst_addr);
-		err = -EPERM;
-		goto cleanup;
-	}
-
-	rec = record_from_string(str);
-	g_free(str);
-
-	err = extract_hid_record(rec, req);
-	sdp_record_free(rec);
+	err = extract_hid_record(idev, req);
 	if (err < 0) {
 		error("Could not parse HID SDP record: %s (%d)", strerror(-err),
 									-err);
@@ -1091,7 +1065,7 @@  static int hidp_add_connection(struct input_device *idev)
 
 	/* Make sure the device is bonded if required */
 	if (classic_bonded_only && !input_device_bonded(idev)) {
-		error("Rejected connection from !bonded device %s", dst_addr);
+		error("Rejected connection from !bonded device %s", idev->path);
 		goto cleanup;
 	}
 
@@ -1161,6 +1135,68 @@  static int connection_disconnect(struct input_device *idev, uint32_t flags)
 		return ioctl_disconnect(idev, flags);
 }
 
+static bool is_device_sdp_disable(const sdp_record_t *rec)
+{
+	sdp_data_t *data;
+
+	data = sdp_data_get(rec, SDP_ATTR_HID_SDP_DISABLE);
+
+	return data && data->val.uint8;
+}
+
+static enum reconnect_mode_t hid_reconnection_mode(bool reconnect_initiate,
+						bool normally_connectable)
+{
+	if (!reconnect_initiate && !normally_connectable)
+		return RECONNECT_NONE;
+	else if (!reconnect_initiate && normally_connectable)
+		return RECONNECT_HOST;
+	else if (reconnect_initiate && !normally_connectable)
+		return RECONNECT_DEVICE;
+	else /* (reconnect_initiate && normally_connectable) */
+		return RECONNECT_ANY;
+}
+
+static void extract_hid_props(struct input_device *idev,
+					const sdp_record_t *rec)
+{
+	/* Extract HID connectability */
+	bool reconnect_initiate, normally_connectable;
+	sdp_data_t *pdlist;
+
+	/* HIDNormallyConnectable is optional and assumed FALSE if not
+	 * present.
+	 */
+	pdlist = sdp_data_get(rec, SDP_ATTR_HID_RECONNECT_INITIATE);
+	reconnect_initiate = pdlist ? pdlist->val.uint8 : TRUE;
+
+	pdlist = sdp_data_get(rec, SDP_ATTR_HID_NORMALLY_CONNECTABLE);
+	normally_connectable = pdlist ? pdlist->val.uint8 : FALSE;
+
+	/* Update local values */
+	idev->reconnect_mode =
+		hid_reconnection_mode(reconnect_initiate, normally_connectable);
+}
+
+static void input_device_update_rec(struct input_device *idev)
+{
+	struct btd_profile *p = btd_service_get_profile(idev->service);
+	const sdp_record_t *rec;
+
+	rec = btd_device_get_record(idev->device, p->remote_uuid);
+	if (!rec || idev->rec == rec)
+		return;
+
+	idev->rec = rec;
+	idev->disable_sdp = is_device_sdp_disable(rec);
+
+	/* Initialize device properties */
+	extract_hid_props(idev, rec);
+
+	if (idev->disable_sdp)
+		device_set_refresh_discovery(idev->device, false);
+}
+
 static int input_device_connected(struct input_device *idev)
 {
 	int err;
@@ -1168,6 +1204,9 @@  static int input_device_connected(struct input_device *idev)
 	if (idev->intr_io == NULL || idev->ctrl_io == NULL)
 		return -ENOTCONN;
 
+	/* Attempt to update SDP record if it had changed */
+	input_device_update_rec(idev);
+
 	err = hidp_add_connection(idev);
 	if (err < 0)
 		return err;
@@ -1411,74 +1450,21 @@  int input_device_disconnect(struct btd_service *service)
 	return 0;
 }
 
-static bool is_device_sdp_disable(const sdp_record_t *rec)
-{
-	sdp_data_t *data;
-
-	data = sdp_data_get(rec, SDP_ATTR_HID_SDP_DISABLE);
-
-	return data && data->val.uint8;
-}
-
-static enum reconnect_mode_t hid_reconnection_mode(bool reconnect_initiate,
-						bool normally_connectable)
-{
-	if (!reconnect_initiate && !normally_connectable)
-		return RECONNECT_NONE;
-	else if (!reconnect_initiate && normally_connectable)
-		return RECONNECT_HOST;
-	else if (reconnect_initiate && !normally_connectable)
-		return RECONNECT_DEVICE;
-	else /* (reconnect_initiate && normally_connectable) */
-		return RECONNECT_ANY;
-}
-
-static void extract_hid_props(struct input_device *idev,
-					const sdp_record_t *rec)
-{
-	/* Extract HID connectability */
-	bool reconnect_initiate, normally_connectable;
-	sdp_data_t *pdlist;
-
-	/* HIDNormallyConnectable is optional and assumed FALSE
-	* if not present. */
-	pdlist = sdp_data_get(rec, SDP_ATTR_HID_RECONNECT_INITIATE);
-	reconnect_initiate = pdlist ? pdlist->val.uint8 : TRUE;
-
-	pdlist = sdp_data_get(rec, SDP_ATTR_HID_NORMALLY_CONNECTABLE);
-	normally_connectable = pdlist ? pdlist->val.uint8 : FALSE;
-
-	/* Update local values */
-	idev->reconnect_mode =
-		hid_reconnection_mode(reconnect_initiate, normally_connectable);
-}
-
 static struct input_device *input_device_new(struct btd_service *service)
 {
 	struct btd_device *device = btd_service_get_device(service);
-	struct btd_profile *p = btd_service_get_profile(service);
 	const char *path = device_get_path(device);
-	const sdp_record_t *rec = btd_device_get_record(device, p->remote_uuid);
 	struct btd_adapter *adapter = device_get_adapter(device);
 	struct input_device *idev;
 
-	if (!rec)
-		return NULL;
-
 	idev = g_new0(struct input_device, 1);
 	bacpy(&idev->src, btd_adapter_get_address(adapter));
 	bacpy(&idev->dst, device_get_address(device));
 	idev->service = btd_service_ref(service);
 	idev->device = btd_device_ref(device);
 	idev->path = g_strdup(path);
-	idev->handle = rec->handle;
-	idev->disable_sdp = is_device_sdp_disable(rec);
 
-	/* Initialize device properties */
-	extract_hid_props(idev, rec);
-
-	if (idev->disable_sdp)
-		device_set_refresh_discovery(device, false);
+	input_device_update_rec(idev);
 
 	return idev;
 }