diff mbox series

[1/2] adapter: Fix addr_type for smp_irk/ltk_info/link_key

Message ID 20231211162729.1183207-1-xiaokeqinhealth@126.com
State New
Headers show
Series [1/2] adapter: Fix addr_type for smp_irk/ltk_info/link_key | expand

Commit Message

Felix Qin Dec. 11, 2023, 4:27 p.m. UTC
From: Xiao Yao <xiaoyao@rock-chips.com>

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 <xiaoyao@rock-chips.com>
---
 src/adapter.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

Comments

Robin Candau Dec. 16, 2023, 12:59 a.m. UTC | #1
On 12/11/23 17:27, Xiao Yao wrote:
> From: Xiao Yao<xiaoyao@rock-chips.com>
>
> 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<xiaoyao@rock-chips.com>
> ---
>   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|
|
diff mbox series

Patch

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);