2023-12-11 16:42:58

by Yao Xiao

[permalink] [raw]
Subject: [PATCH 1/2] adapter: Fix addr_type for smp_irk/ltk_info/link_key

From: Xiao Yao <[email protected]>

According to BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 3,
Part H, 2.4.24/2.4.25, The LE LTK and BR/EDR link keys can be
converted between each other, so the addr type can be either
BREDR or LE, so fix it.

Signed-off-by: Xiao Yao <[email protected]>
---
src/adapter.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 86fff72bc..ee70b00d2 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -170,6 +170,7 @@ static GSList *conn_fail_list = NULL;

struct link_key_info {
bdaddr_t bdaddr;
+ uint8_t bdaddr_type;
unsigned char key[16];
uint8_t type;
uint8_t pin_len;
@@ -3964,7 +3965,9 @@ static bool is_blocked_key(uint8_t key_type, uint8_t *key_value)
return false;
}

-static struct link_key_info *get_key_info(GKeyFile *key_file, const char *peer)
+static struct link_key_info *get_key_info(GKeyFile *key_file, const char *peer,
+ uint8_t bdaddr_type)
+
{
struct link_key_info *info = NULL;
char *str;
@@ -3976,6 +3979,7 @@ static struct link_key_info *get_key_info(GKeyFile *key_file, const char *peer)
info = g_new0(struct link_key_info, 1);

str2ba(peer, &info->bdaddr);
+ info->bdaddr_type = bdaddr_type;

if (!strncmp(str, "0x", 2))
str2buf(&str[2], info->key, sizeof(info->key));
@@ -4343,7 +4347,7 @@ 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 = BDADDR_BREDR;
+ key->addr.type = info->bdaddr_type;
key->type = info->type;
memcpy(key->val, info->key, 16);
key->pin_len = info->pin_len;
@@ -4598,14 +4602,18 @@ static void load_conn_params(struct btd_adapter *adapter, GSList *params)
btd_error(adapter->dev_id, "Load connection parameters failed");
}

-static uint8_t get_le_addr_type(GKeyFile *keyfile)
+static uint8_t get_addr_type(GKeyFile *keyfile)
{
uint8_t addr_type;
char *type;

+ /* The AddressType is written to file only When dev->le is
+ * set to true, as referenced in the update_technologies().
+ * Therefore, When type is NULL, it default to BDADDR_BREDR.
+ */
type = g_key_file_get_string(keyfile, "General", "AddressType", NULL);
if (!type)
- return BDADDR_LE_PUBLIC;
+ return BDADDR_BREDR;

if (g_str_equal(type, "public"))
addr_type = BDADDR_LE_PUBLIC;
@@ -4914,9 +4922,9 @@ static void load_devices(struct btd_adapter *adapter)
g_clear_error(&gerr);
}

- key_info = get_key_info(key_file, entry->d_name);
+ bdaddr_type = get_addr_type(key_file);

- bdaddr_type = get_le_addr_type(key_file);
+ key_info = get_key_info(key_file, entry->d_name, bdaddr_type);

ltk_info = get_ltk_info(key_file, entry->d_name, bdaddr_type);

--
2.34.1



2023-12-11 18:18:07

by bluez.test.bot

[permalink] [raw]
Subject: RE: [1/2] adapter: Fix addr_type for smp_irk/ltk_info/link_key

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

---Test result---

Test Summary:
CheckPatch PASS 0.98 seconds
GitLint PASS 0.64 seconds
BuildEll PASS 24.31 seconds
BluezMake PASS 717.35 seconds
MakeCheck PASS 11.35 seconds
MakeDistcheck PASS 157.03 seconds
CheckValgrind PASS 220.60 seconds
CheckSmatch PASS 329.27 seconds
bluezmakeextell PASS 103.85 seconds
IncrementalBuild PASS 1351.13 seconds
ScanBuild PASS 930.85 seconds



---
Regards,
Linux Bluetooth

2023-12-11 18:41:49

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH 1/2] adapter: Fix addr_type for smp_irk/ltk_info/link_key

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Tue, 12 Dec 2023 00:27:28 +0800 you wrote:
> From: Xiao Yao <[email protected]>
>
> According to BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 3,
> Part H, 2.4.24/2.4.25, The LE LTK and BR/EDR link keys can be
> converted between each other, so the addr type can be either
> BREDR or LE, so fix it.
>
> [...]

Here is the summary with links:
- [1/2] adapter: Fix addr_type for smp_irk/ltk_info/link_key
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d5536e0cd431
- [2/2] device: Add notifications when the bdaddr_type changes
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ba9fda12d26b

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



2023-12-16 01:06:21

by Robin Candau

[permalink] [raw]
Subject: Re: [PATCH 1/2] adapter: Fix addr_type for smp_irk/ltk_info/link_key

On 12/11/23 17:27, Xiao Yao wrote:
> From: Xiao Yao<[email protected]>
>
> According to BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 3,
> Part H, 2.4.24/2.4.25, The LE LTK and BR/EDR link keys can be
> converted between each other, so the addr type can be either
> BREDR or LE, so fix it.
>
> Signed-off-by: Xiao Yao<[email protected]>
> ---
> src/adapter.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 86fff72bc..ee70b00d2 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -170,6 +170,7 @@ static GSList *conn_fail_list = NULL;
>
> struct link_key_info {
> bdaddr_t bdaddr;
> + uint8_t bdaddr_type;
> unsigned char key[16];
> uint8_t type;
> uint8_t pin_len;
> @@ -3964,7 +3965,9 @@ static bool is_blocked_key(uint8_t key_type, uint8_t *key_value)
> return false;
> }
>
> -static struct link_key_info *get_key_info(GKeyFile *key_file, const char *peer)
> +static struct link_key_info *get_key_info(GKeyFile *key_file, const char *peer,
> + uint8_t bdaddr_type)
> +
> {
> struct link_key_info *info = NULL;
> char *str;
> @@ -3976,6 +3979,7 @@ static struct link_key_info *get_key_info(GKeyFile *key_file, const char *peer)
> info = g_new0(struct link_key_info, 1);
>
> str2ba(peer, &info->bdaddr);
> + info->bdaddr_type = bdaddr_type;
>
> if (!strncmp(str, "0x", 2))
> str2buf(&str[2], info->key, sizeof(info->key));
> @@ -4343,7 +4347,7 @@ 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 = BDADDR_BREDR;
> + key->addr.type = info->bdaddr_type;
> key->type = info->type;
> memcpy(key->val, info->key, 16);
> key->pin_len = info->pin_len;
> @@ -4598,14 +4602,18 @@ static void load_conn_params(struct btd_adapter *adapter, GSList *params)
> btd_error(adapter->dev_id, "Load connection parameters failed");
> }
>
> -static uint8_t get_le_addr_type(GKeyFile *keyfile)
> +static uint8_t get_addr_type(GKeyFile *keyfile)
> {
> uint8_t addr_type;
> char *type;
>
> + /* The AddressType is written to file only When dev->le is
> + * set to true, as referenced in the update_technologies().
> + * Therefore, When type is NULL, it default to BDADDR_BREDR.
> + */
> type = g_key_file_get_string(keyfile, "General", "AddressType", NULL);
> if (!type)
> - return BDADDR_LE_PUBLIC;
> + return BDADDR_BREDR;
>
> if (g_str_equal(type, "public"))
> addr_type = BDADDR_LE_PUBLIC;
> @@ -4914,9 +4922,9 @@ static void load_devices(struct btd_adapter *adapter)
> g_clear_error(&gerr);
> }
>
> - key_info = get_key_info(key_file, entry->d_name);
> + bdaddr_type = get_addr_type(key_file);
>
> - bdaddr_type = get_le_addr_type(key_file);
> + key_info = get_key_info(key_file, entry->d_name, bdaddr_type);
>
> ltk_info = get_ltk_info(key_file, entry->d_name, bdaddr_type);
>

Hello,

It seems like the above commit introduced a regression where the initial
auto-connect for Bluetooth devices doesn't work anymore.

Indeed, at system startup, starting a Bluetooth device will cause it to
go in a "connected/disconnected" state loop, requiring to initialize a
manual connection first (with sometimes multiple attempts needed) before
getting it connected correctly and working as intended.

`systemctl status bluetooth` reports the following error:

[...]
déc. 15 11:03:18 Arch-Desktop bluetoothd[592]: Failed to load link keys
for hci0: Invalid Parameters (0x0d)
[...]

**I bisected the bug with `git bisect` and it pointed out this commit
[1] as the "faulty" one.
I can confirm that reverting it solves the issue.

I reported this bug including details in the GitHub repo [2].

I remain available if any additional information are needed.

[1]
https://github.com/bluez/bluez/commit/d5536e0cd431e22be9a1132be6d4af2445590633
[2] https://github.com/bluez/bluez/issues/686|
|

--
Regards,
Robin Candau / Antiz


Attachments:
OpenPGP_0xFDC3040B92ACA748.asc (3.13 kB)
OpenPGP public key
OpenPGP_signature.asc (855.00 B)
OpenPGP digital signature
Download all attachments