bluetooth: remove blocking operation in bt_conn_get_info #95600

liuX10 · 2025-09-06T13:16:38Z

bt_conn_get_info API is used to retrieve connection-related information. However, bt_conn_get_info sends the HCI command BT_HCI_OP_READ_ENCRYPTION_KEY_SIZE to retrieve current key_size, causing excessive blocking time.

github-actions · 2025-09-06T13:17:21Z

Hello @liuX10, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

Thalley · 2025-09-06T17:26:33Z

subsys/bluetooth/host/keys.h

 struct bt_keys_link_key {
 	bt_addr_t addr;
 	uint8_t storage_start[0] __aligned(sizeof(void *));
+	uint8_t key_size;


Correct me if I'm wrong, but wouldn't this affect BT_KEYS_LINK_KEY_STORAGE_LEN, and thus this change would basically invalidate/break any currently stored keys in persistent storage?

@Thalley Yes, you are right.

With the changes in this PR, the key size does not need to be stored in persistent storage, as it is only used to store the encrypted key size.

While, for the follow-up, if there is a scenario where the previously encrypted key size needs to be used, we will also encounter the situation where BT_KEYS_LINK_KEY_STORAGE_LEN is modified. For this case, how it is processed in Zephyr?

With the changes in this PR, the key size does not need to be stored in persistent storage, as it is only used to store the encrypted key size.

In that case, I think the key_size should be moved above storage_start

While, for the follow-up, if there is a scenario where the previously encrypted key size needs to be used, we will also encounter the situation where BT_KEYS_LINK_KEY_STORAGE_LEN is modified. For this case, how it is processed in Zephyr?

Not sure I follow this - Can you elaborate?

Yes, moving key_size above storage_start can prevent decoding errors when load stored keys.

@liuX10 This also means that we don't store the key_size in flash.

When we re-read the keys, then we won't know the size of the key:

memcpy(link_key->storage_start, val, len); LOG_DBG("Successfully restored link key for %s", bt_addr_le_str(&le_addr));

But I think this is also an issue today, so perhaps that needs to be followed up with another PR?

In comparison, have a look at the BT LE keys:

struct bt_keys { uint8_t id; bt_addr_le_t addr; uint8_t state; uint8_t storage_start[0] __aligned(sizeof(void *)); uint8_t enc_size; uint8_t flags; uint16_t keys; struct bt_ltk ltk; struct bt_irk irk; #if defined(CONFIG_BT_SIGNING) struct bt_csrk local_csrk; struct bt_csrk remote_csrk; #endif /* BT_SIGNING */ #if !defined(CONFIG_BT_SMP_SC_PAIR_ONLY) struct bt_ltk periph_ltk; #endif /* CONFIG_BT_SMP_SC_PAIR_ONLY */ #if (defined(CONFIG_BT_KEYS_OVERWRITE_OLDEST)) uint32_t aging_counter; #endif /* CONFIG_BT_KEYS_OVERWRITE_OLDEST */ };

Here the enc_size is part of what we store in flash (defined after storage_start), so when we restore the keys from flash, we know the actual size of the key.

So I think you need to put key_size after storage_start to support this correctly, but that will, as my original comment pointed out, probably invalidate all existing BT Classic settings

The link key is always 128bits. And the encryption key size is negotiated. And the range is 8bits ~ 128 bits. So the encryption key size only can be got when ACL connection is established.

I think the enc_key_size is 0 when calling get_info, if the ACL is not encrypted. And if the encrypt changed or key refreshed event notified, the enc key size can be got from HCI_Encryption_Change[v2] or HCI_Read_Encryption_Key_Size. And it is kept into struct bt_keys::enc_size.
And the enc_key_size is struct bt_keys::enc_size when calling get_info if the ACL is encrypted.

The link key is always 128bits. And the encryption key size is negotiated. And the range is 8bits ~ 128 bits. So the encryption key size only can be got when ACL connection is established.

Ah, given the naming, I thought bt_keys_link_key.key_size referred to the link key size, but it sounds like it refers to the encryption key size (which makes sense). If the link key size is always 128 bits, then I think everything here makes sense :) But perhaps bt_keys_link_key.key_size should be updated to enc_key_size to avoid confusion (for people like me who are not experts in BT Classic) - Perhaps with an additional comment that the key size will be updated once the link is encrypted, and is otherwise 0.

One followup question: With the same link key, can the encryption key size change between connections? i.e. can the following happen:

Connect 2 devices. Encryption key size is 80 bits.

Devices disconnect

Connect the same 2 devices. Encryption key size is 88 bits

Just wondering if conn->br.link_key->key_size should be set to 0 on disconnect

Perhaps with an additional comment that the key size will be updated once the link is encrypted, and is otherwise 0.

Yes, I think so.

Connect 2 devices. Encryption key size is 80 bits.

Devices disconnect

Connect the same 2 devices. Encryption key size is 88 bits

I did not notice that. I think it depends on the implementation of controller.

Just wondering if conn->br.link_key->key_size should be set to 0 on disconnect

Yes, I think it is reasonable.

@liuX10 Could you please update the PR to address these comments?

Sounds like we are in agreement

So add comment/rename to make it clear that it is the size of the encryption key, and not the link key, and reset the encryption key size to 0 on disconnect, as it may change between connections, and storing the encryption key size from the last connection may be confusing

okay! so I need to rename key_size as enc_key_size, and reset enc_key_size to 0 as disconnect.

liuX10 · 2025-09-15T13:58:40Z

subsys/bluetooth/host/hci_core.c

 		 * database row for this connection.
 		 */
 		if (conn->type == BT_CONN_TYPE_BR && conn->br.link_key != NULL &&
 		    atomic_test_and_clear_bit(conn->flags, BT_CONN_BR_NOBOND)) {


@Thalley I found that there is an additional condition in this conditional statement that may prevent enc_key_size from being set to 0.
atomic_test_and_clear_bit(conn->flags, BT_CONN_BR_NOBOND)

Hmm, my understanding from #95600 (comment) was that the encryption key size could be different between connections, even when we are bonded.

i.e. even if we are bonded, we should set the encryption key size to 0, which the current patch you just pushed seemingly doesn't

@Thalley I guess you might have misunderstood the meaning of BT_CONN_BR_NOBOND. This flag means pair but not bond.
Previous changes may exist. In bond mode, enc_key_size is not reset to 0 as disconnect; it is only set to 0 in not bond mode alongside Linkkey clear.

if (conn->type == BT_CONN_TYPE_BR && conn->br.link_key != NULL && atomic_test_and_clear_bit(conn->flags, BT_CONN_BR_NOBOND))

now, current push ensures that the reset of enc_key_size is no longer tied to BOND mode or non-BOND mode.

if (conn->type == BT_CONN_TYPE_BR && conn->br.link_key != NULL) { if (atomic_test_and_clear_bit(conn->flags, BT_CONN_BR_NOBOND)) bt_keys_link_key_clear(conn->br.link_key); conn->br.link_key->enc_key_size = 0; }

jhedberg · 2025-09-29T09:47:36Z

@liuX10 you still need to fix the Compliance check. It seems like the git author doesn't match the signed-off-by line. You should be able to fix that with git commit --amend --reset-author (assuming your git configuration contains the right info)

liuX10 · 2025-10-09T01:57:30Z

Hi! @jhedberg
Could you help merge this patch?

Thalley · 2025-10-09T10:04:48Z

Hi! @jhedberg Could you help merge this patch?

@lylezhu2012 can also approve as the assignee :)

jhedberg · 2025-10-10T12:18:09Z

@liuX10 there's a Compliance check failure that still needs fixing

bt_conn_get_info API is used to retrieve connection-related information. However, bt_conn_get_info sends the HCI command BT_HCI_OP_READ_ENCRYPTION_KEY_SIZE to retrieve current key_size, causing excessive blocking time. Signed-off-by: Xiang Liu <[email protected]>

sonarqubecloud · 2025-10-14T02:59:28Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

liuX10 · 2025-10-14T10:32:32Z

@liuX10 there's a Compliance check failure that still needs fixing

@jhedberg hi, help to merge this patch? Thanks

github-actions · 2025-10-14T11:45:41Z

Hi @liuX10!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

zephyrbot added area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth Classic Bluetooth Classic (BR/EDR) area: Bluetooth labels Sep 6, 2025

zephyrbot requested review from HaavardRei, MarkWangChinese, PavelVPV, Thalley, alwa-nordic, cvinayak, hermabe, jhedberg, lylezhu2012, makeshi, rugeGerritsen and sjanc September 6, 2025 13:17

zephyrbot assigned jhedberg and alwa-nordic Sep 6, 2025

jhedberg previously approved these changes Sep 6, 2025

View reviewed changes

Thalley requested changes Sep 6, 2025

View reviewed changes

liuX10 dismissed jhedberg’s stale review via d6eb7c4 September 12, 2025 08:32

liuX10 force-pushed the main branch from e7bfbf0 to d6eb7c4 Compare September 12, 2025 08:32

liuX10 requested review from Thalley and jhedberg September 12, 2025 08:38

alwa-nordic removed their assignment Sep 15, 2025

liuX10 force-pushed the main branch from d6eb7c4 to e3ae28c Compare September 15, 2025 13:38

Thalley previously approved these changes Sep 15, 2025

View reviewed changes

liuX10 commented Sep 15, 2025

View reviewed changes

liuX10 dismissed Thalley’s stale review via aa04dd4 September 15, 2025 13:59

liuX10 force-pushed the main branch from e3ae28c to aa04dd4 Compare September 15, 2025 13:59

kartben force-pushed the main branch from 5f8022b to 9d6db1c Compare September 30, 2025 08:41

liuX10 dismissed stale reviews from gzh-terry and Thalley via 848f5f4 October 10, 2025 08:34

liuX10 force-pushed the main branch from 9d6db1c to 848f5f4 Compare October 10, 2025 08:34

liuX10 requested review from Thalley, chengkai15 and gzh-terry October 10, 2025 08:35

Thalley approved these changes Oct 10, 2025

View reviewed changes

lylezhu2012 approved these changes Oct 10, 2025

View reviewed changes

liuX10 dismissed chengkai15’s stale review via 848f5f4 October 10, 2025 12:17

jhedberg approved these changes Oct 10, 2025

View reviewed changes

liuX10 force-pushed the main branch from 848f5f4 to 4ce6935 Compare October 10, 2025 14:10

zephyrbot requested review from jhedberg and lylezhu2012 October 10, 2025 14:12

chengkai15 approved these changes Oct 10, 2025

View reviewed changes

liuX10 force-pushed the main branch 3 times, most recently from 1d09429 to 4f60807 Compare October 14, 2025 02:50

liuX10 force-pushed the main branch from 4f60807 to aa42f28 Compare October 14, 2025 02:55

liuX10 requested review from Thalley and chengkai15 October 14, 2025 10:31

cfriedt merged commit 8e9fa6a into zephyrproject-rtos:main Oct 14, 2025
29 checks passed

bluetooth: remove blocking operation in bt_conn_get_info #95600

bluetooth: remove blocking operation in bt_conn_get_info #95600

Uh oh!

Conversation

liuX10 commented Sep 6, 2025

Uh oh!

github-actions bot commented Sep 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lylezhu2012 Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lylezhu2012 Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liuX10 Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liuX10 Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhedberg commented Sep 29, 2025

Uh oh!

liuX10 commented Oct 9, 2025

Uh oh!

Thalley commented Oct 9, 2025

Uh oh!

jhedberg commented Oct 10, 2025

Uh oh!

sonarqubecloud bot commented Oct 14, 2025

Quality Gate passed

Uh oh!

liuX10 commented Oct 14, 2025

Uh oh!

Uh oh!

github-actions bot commented Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

lylezhu2012 Sep 8, 2025 •

edited

Loading

lylezhu2012 Sep 15, 2025 •

edited

Loading

liuX10 Sep 15, 2025 •

edited

Loading

liuX10 Sep 16, 2025 •

edited

Loading