From: Luiz Augusto von Dentz <[email protected]>
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(-)
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;
}
--
2.41.0
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
Hello:
This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Tue, 10 Oct 2023 12:43:06 -0700 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> 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.
>
> [...]
Here is the summary with links:
- [BlueZ] input: Fix .device_probe failing if SDP record is not found
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=3a9c637010f8
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html