From: Xiao Yao <[email protected]>
According to the Bluetooth specification, the address
type of the link key is not fixed. However, the
load_link_keys function in the old kernel code requires
that the address type must be BDADDR_BREDR, so attempt
it when the first load fails.
Fixes: https://github.com/bluez/bluez/issues/686
Signed-off-by: Xiao Yao <[email protected]>
---
v1 -> v2
Prioritize loading keys with standard address types,
and switch to BREDR address types upon failure, as
suggested by the maintainer.
---
src/adapter.c | 45 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index ee70b00d2..e1b704ecc 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -168,6 +168,9 @@ static GSList *adapter_drivers = NULL;
static GSList *disconnect_list = NULL;
static GSList *conn_fail_list = NULL;
+static GSList *link_keys = NULL;
+static bool link_keys_brder = false;
+
struct link_key_info {
bdaddr_t bdaddr;
uint8_t bdaddr_type;
@@ -4284,23 +4287,45 @@ static int set_privacy(struct btd_adapter *adapter, uint8_t privacy)
return -1;
}
+static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
+ bool debug_keys, bool link_key_bredr);
+
static void load_link_keys_complete(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
struct btd_adapter *adapter = user_data;
if (status != MGMT_STATUS_SUCCESS) {
+ /*
+ * According to the Bluetooth specification, the address
+ * type of the link key is not fixed. However, the
+ * load_link_keys function in the old kernel code requires
+ * that the address type must be BDADDR_BREDR, so attempt it.
+ */
+ if (link_keys_brder == false && status == 0x0d) {
+ link_keys_brder = true;
+ load_link_keys(adapter, link_keys, btd_opts.debug_keys,
+ link_keys_brder);
+ return;
+ }
+
btd_error(adapter->dev_id,
"Failed to load link keys for hci%u: %s (0x%02x)",
adapter->dev_id, mgmt_errstr(status), status);
- return;
+
+ goto free;
}
DBG("link keys loaded for hci%u", adapter->dev_id);
+
+free:
+ g_slist_free_full(link_keys, g_free);
+ link_keys = NULL;
+ link_keys_brder = false;
}
static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
- bool debug_keys)
+ bool debug_keys, bool link_key_bredr)
{
struct mgmt_cp_load_link_keys *cp;
struct mgmt_link_key_info *key;
@@ -4320,8 +4345,8 @@ static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
key_count = g_slist_length(keys);
- DBG("hci%u keys %zu debug_keys %d", adapter->dev_id, key_count,
- debug_keys);
+ DBG("hci%u keys %zu debug_keys %d (%s)", adapter->dev_id, key_count,
+ debug_keys, link_key_bredr ? "force bredr" : "normal");
cp_size = sizeof(*cp) + (key_count * sizeof(*key));
@@ -4347,7 +4372,10 @@ static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
struct link_key_info *info = l->data;
bacpy(&key->addr.bdaddr, &info->bdaddr);
- key->addr.type = info->bdaddr_type;
+ if (link_key_bredr)
+ key->addr.type = BDADDR_BREDR;
+ else
+ key->addr.type = info->bdaddr_type;
key->type = info->type;
memcpy(key->val, info->key, 16);
key->pin_len = info->pin_len;
@@ -4873,7 +4901,6 @@ done:
static void load_devices(struct btd_adapter *adapter)
{
char dirname[PATH_MAX];
- GSList *keys = NULL;
GSList *ltks = NULL;
GSList *irks = NULL;
GSList *params = NULL;
@@ -4964,7 +4991,7 @@ static void load_devices(struct btd_adapter *adapter)
}
if (key_info)
- keys = g_slist_append(keys, key_info);
+ link_keys = g_slist_append(link_keys, key_info);
if (ltk_info)
ltks = g_slist_append(ltks, ltk_info);
@@ -5013,8 +5040,8 @@ free:
closedir(dir);
- load_link_keys(adapter, keys, btd_opts.debug_keys);
- g_slist_free_full(keys, g_free);
+ load_link_keys(adapter, link_keys, btd_opts.debug_keys,
+ link_keys_brder);
load_ltks(adapter, ltks);
g_slist_free_full(ltks, g_free);
--
2.34.1
Hi Xiao,
On Wed, Dec 20, 2023 at 12:31 PM Xiao Yao <[email protected]> wrote:
>
> From: Xiao Yao <[email protected]>
>
> According to the Bluetooth specification, the address
> type of the link key is not fixed. However, the
> load_link_keys function in the old kernel code requires
> that the address type must be BDADDR_BREDR, so attempt
> it when the first load fails.
>
> Fixes: https://github.com/bluez/bluez/issues/686
>
> Signed-off-by: Xiao Yao <[email protected]>
> ---
> v1 -> v2
> Prioritize loading keys with standard address types,
> and switch to BREDR address types upon failure, as
> suggested by the maintainer.
> ---
> src/adapter.c | 45 ++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index ee70b00d2..e1b704ecc 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -168,6 +168,9 @@ static GSList *adapter_drivers = NULL;
> static GSList *disconnect_list = NULL;
> static GSList *conn_fail_list = NULL;
>
> +static GSList *link_keys = NULL;
> +static bool link_keys_brder = false;
> +
> struct link_key_info {
> bdaddr_t bdaddr;
> uint8_t bdaddr_type;
> @@ -4284,23 +4287,45 @@ static int set_privacy(struct btd_adapter *adapter, uint8_t privacy)
> return -1;
> }
>
> +static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
> + bool debug_keys, bool link_key_bredr);
> +
> static void load_link_keys_complete(uint8_t status, uint16_t length,
> const void *param, void *user_data)
> {
> struct btd_adapter *adapter = user_data;
>
> if (status != MGMT_STATUS_SUCCESS) {
> + /*
> + * According to the Bluetooth specification, the address
> + * type of the link key is not fixed. However, the
> + * load_link_keys function in the old kernel code requires
> + * that the address type must be BDADDR_BREDR, so attempt it.
> + */
> + if (link_keys_brder == false && status == 0x0d) {
> + link_keys_brder = true;
> + load_link_keys(adapter, link_keys, btd_opts.debug_keys,
> + link_keys_brder);
> + return;
> + }
> +
> btd_error(adapter->dev_id,
> "Failed to load link keys for hci%u: %s (0x%02x)",
> adapter->dev_id, mgmt_errstr(status), status);
> - return;
> +
> + goto free;
> }
>
> DBG("link keys loaded for hci%u", adapter->dev_id);
> +
> +free:
> + g_slist_free_full(link_keys, g_free);
> + link_keys = NULL;
> + link_keys_brder = false;
> }
>
> static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
> - bool debug_keys)
> + bool debug_keys, bool link_key_bredr)
> {
> struct mgmt_cp_load_link_keys *cp;
> struct mgmt_link_key_info *key;
> @@ -4320,8 +4345,8 @@ static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
>
> key_count = g_slist_length(keys);
>
> - DBG("hci%u keys %zu debug_keys %d", adapter->dev_id, key_count,
> - debug_keys);
> + DBG("hci%u keys %zu debug_keys %d (%s)", adapter->dev_id, key_count,
> + debug_keys, link_key_bredr ? "force bredr" : "normal");
>
> cp_size = sizeof(*cp) + (key_count * sizeof(*key));
>
> @@ -4347,7 +4372,10 @@ static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
> struct link_key_info *info = l->data;
>
> bacpy(&key->addr.bdaddr, &info->bdaddr);
> - key->addr.type = info->bdaddr_type;
> + if (link_key_bredr)
> + key->addr.type = BDADDR_BREDR;
> + else
> + key->addr.type = info->bdaddr_type;
> key->type = info->type;
> memcpy(key->val, info->key, 16);
> key->pin_len = info->pin_len;
> @@ -4873,7 +4901,6 @@ done:
> static void load_devices(struct btd_adapter *adapter)
> {
> char dirname[PATH_MAX];
> - GSList *keys = NULL;
> GSList *ltks = NULL;
> GSList *irks = NULL;
> GSList *params = NULL;
> @@ -4964,7 +4991,7 @@ static void load_devices(struct btd_adapter *adapter)
> }
>
> if (key_info)
> - keys = g_slist_append(keys, key_info);
> + link_keys = g_slist_append(link_keys, key_info);
>
> if (ltk_info)
> ltks = g_slist_append(ltks, ltk_info);
> @@ -5013,8 +5040,8 @@ free:
>
> closedir(dir);
>
> - load_link_keys(adapter, keys, btd_opts.debug_keys);
> - g_slist_free_full(keys, g_free);
> + load_link_keys(adapter, link_keys, btd_opts.debug_keys,
> + link_keys_brder);
>
> load_ltks(adapter, ltks);
> g_slist_free_full(ltks, g_free);
> --
> 2.34.1
Ive just sent a similar fix:
https://patchwork.kernel.org/project/bluetooth/patch/[email protected]/
>
--
Luiz Augusto von Dentz
Hi Luiz,
Thank you very much for fixing this issue; please disregard this patch.
Best Regards
>On Wed, Dec 20, 2023 at 12:31 PM Xiao Yao <[email protected]> wrote:
>>
>> From: Xiao Yao <[email protected]>
>>
>> According to the Bluetooth specification, the address
>> type of the link key is not fixed. However, the
>> load_link_keys function in the old kernel code requires
>> that the address type must be BDADDR_BREDR, so attempt
>> it when the first load fails.
>>
>> Fixes: https://github.com/bluez/bluez/issues/686
>>
>> Signed-off-by: Xiao Yao <[email protected]>
>> ---
>> v1 -> v2
>> Prioritize loading keys with standard address types,
>> and switch to BREDR address types upon failure, as
>> suggested by the maintainer.
>> ---
>> src/adapter.c | 45 ++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 36 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/adapter.c b/src/adapter.c
>> index ee70b00d2..e1b704ecc 100644
>> --- a/src/adapter.c
>> +++ b/src/adapter.c
>> @@ -168,6 +168,9 @@ static GSList *adapter_drivers = NULL;
>> static GSList *disconnect_list = NULL;
>> static GSList *conn_fail_list = NULL;
>>
>> +static GSList *link_keys = NULL;
>> +static bool link_keys_brder = false;
>> +
>> struct link_key_info {
>> bdaddr_t bdaddr;
>> uint8_t bdaddr_type;
>> @@ -4284,23 +4287,45 @@ static int set_privacy(struct btd_adapter *adapter, uint8_t privacy)
>> return -1;
>> }
>>
>> +static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
>> + bool debug_keys, bool link_key_bredr);
>> +
>> static void load_link_keys_complete(uint8_t status, uint16_t length,
>> const void *param, void *user_data)
>> {
>> struct btd_adapter *adapter = user_data;
>>
>> if (status != MGMT_STATUS_SUCCESS) {
>> + /*
>> + * According to the Bluetooth specification, the address
>> + * type of the link key is not fixed. However, the
>> + * load_link_keys function in the old kernel code requires
>> + * that the address type must be BDADDR_BREDR, so attempt it.
>> + */
>> + if (link_keys_brder == false && status == 0x0d) {
>> + link_keys_brder = true;
>> + load_link_keys(adapter, link_keys, btd_opts.debug_keys,
>> + link_keys_brder);
>> + return;
>> + }
>> +
>> btd_error(adapter->dev_id,
>> "Failed to load link keys for hci%u: %s (0x%02x)",
>> adapter->dev_id, mgmt_errstr(status), status);
>> - return;
>> +
>> + goto free;
>> }
>>
>> DBG("link keys loaded for hci%u", adapter->dev_id);
>> +
>> +free:
>> + g_slist_free_full(link_keys, g_free);
>> + link_keys = NULL;
>> + link_keys_brder = false;
>> }
>>
>> static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
>> - bool debug_keys)
>> + bool debug_keys, bool link_key_bredr)
>> {
>> struct mgmt_cp_load_link_keys *cp;
>> struct mgmt_link_key_info *key;
>> @@ -4320,8 +4345,8 @@ static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
>>
>> key_count = g_slist_length(keys);
>>
>> - DBG("hci%u keys %zu debug_keys %d", adapter->dev_id, key_count,
>> - debug_keys);
>> + DBG("hci%u keys %zu debug_keys %d (%s)", adapter->dev_id, key_count,
>> + debug_keys, link_key_bredr ? "force bredr" : "normal");
>>
>> cp_size = sizeof(*cp) + (key_count * sizeof(*key));
>>
>> @@ -4347,7 +4372,10 @@ static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
>> struct link_key_info *info = l->data;
>>
>> bacpy(&key->addr.bdaddr, &info->bdaddr);
>> - key->addr.type = info->bdaddr_type;
>> + if (link_key_bredr)
>> + key->addr.type = BDADDR_BREDR;
>> + else
>> + key->addr.type = info->bdaddr_type;
>> key->type = info->type;
>> memcpy(key->val, info->key, 16);
>> key->pin_len = info->pin_len;
>> @@ -4873,7 +4901,6 @@ done:
>> static void load_devices(struct btd_adapter *adapter)
>> {
>> char dirname[PATH_MAX];
>> - GSList *keys = NULL;
>> GSList *ltks = NULL;
>> GSList *irks = NULL;
>> GSList *params = NULL;
>> @@ -4964,7 +4991,7 @@ static void load_devices(struct btd_adapter *adapter)
>> }
>>
>> if (key_info)
>> - keys = g_slist_append(keys, key_info);
>> + link_keys = g_slist_append(link_keys, key_info);
>>
>> if (ltk_info)
>> ltks = g_slist_append(ltks, ltk_info);
>> @@ -5013,8 +5040,8 @@ free:
>>
>> closedir(dir);
>>
>> - load_link_keys(adapter, keys, btd_opts.debug_keys);
>> - g_slist_free_full(keys, g_free);
>> + load_link_keys(adapter, link_keys, btd_opts.debug_keys,
>> + link_keys_brder);
>>
>> load_ltks(adapter, ltks);
>> g_slist_free_full(ltks, g_free);
>> --
>> 2.34.1
>
>Ive just sent a similar fix:
>
>https://patchwork.kernel.org/project/bluetooth/patch/[email protected]/
>
>>
>
>
>--
>Luiz Augusto von Dentz
>
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=811904
---Test result---
Test Summary:
CheckPatch FAIL 0.71 seconds
GitLint PASS 0.31 seconds
BuildEll PASS 24.24 seconds
BluezMake PASS 727.82 seconds
MakeCheck PASS 12.00 seconds
MakeDistcheck PASS 160.13 seconds
CheckValgrind PASS 222.20 seconds
CheckSmatch PASS 326.17 seconds
bluezmakeextell PASS 106.33 seconds
IncrementalBuild PASS 674.29 seconds
ScanBuild PASS 925.41 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[v2] adapter: Fix link key address type for old kernels
ERROR:INITIALISED_STATIC: do not initialise statics to NULL
#76: FILE: src/adapter.c:171:
+static GSList *link_keys = NULL;
ERROR:INITIALISED_STATIC: do not initialise statics to false
#77: FILE: src/adapter.c:172:
+static bool link_keys_brder = false;
/github/workspace/src/src/13500412.patch total: 2 errors, 0 warnings, 102 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/src/13500412.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