fix(nordic/ble): fix connection interval setting
What the developer wrote
Authored by tychovrahe
fix(nordic/ble): fix connection interval setting
[no changelog]
What changed, and why it matters
This commit fixes how the Trezor hardware wallet's Bluetooth Low Energy (BLE) connection settings are changed. It adds a mutex (a kind of traffic light) around code that updates BLE connection parameters, and moves two update calls so they happen only after the connection is safely recorded. Without the mutex, two pieces of code could try to change the connection settings at the same time, which on this Nordic BLE stack can cause the update request to fail or behave unpredictably. The commit message does not call this a security fix, but race conditions in connection management can in principle be abused to disrupt or confuse the BLE link.
Treat as a stability/reliability fix rather than a confirmed security vulnerability. Include in routine firmware updates. If a security review of the BLE stack is underway, verify that other concurrent accesses to `current_conn`/`next_conn` are similarly synchronized, and that `connection_update_params()` does not perform blocking operations while holding the mutex.
Security signals we found
Race condition in BLE connection parameter updates mitigated by mutex
Connection update now occurs after `current_conn` is assigned, reducing window for stale/NULL connection use
No explicit security framing in commit message or changelog
No CVE, advisory, or researcher attribution present in supplied materials
Evidence from the diff
The patch introduces a static mutex conn_mutex around calls to connection_update_params() and bt_conn_le_param_update() in nordic/trezor/trezor-ble/src/ble/connection.c. Previously, connected() invoked connection_update_params() and ble_reconfigure_tx_power() before assigning current_conn. The patch moves those calls after current_conn is set (and next_conn handled), and wraps parameter-update paths in connection_suspend(), connection_resume(), connection_set_high_speed(), and connection_set_low_speed() with k_mutex_lock/unlock(). This prevents concurrent updates to the same bt_conn object. The commit is tagged [no changelog] and the message frames it as a connection-interval bug fix, not a security vulnerability.
Changed components
nordic/trezor/trezor-ble/src/ble/connection.cTrezor BLE connection management layerNordic Zephyr BLE host stack connection parameter update pathInspect captured patch +25 / −5
diff --git a/nordic/trezor/trezor-ble/src/ble/connection.c b/nordic/trezor/trezor-ble/src/ble/connection.c
index 73a06c4bb..514ac7c9a 100644
--- a/nordic/trezor/trezor-ble/src/ble/connection.c
+++ b/nordic/trezor/trezor-ble/src/ble/connection.c
@@ -36,6 +36,8 @@ LOG_MODULE_REGISTER(LOG_MODULE_NAME);
#define PPCP_HIGH_SPEED BT_LE_CONN_PARAM(12, 12, 0, 400)
#define PPCP_LOW_SPEED BT_LE_CONN_PARAM(24, 36, 0, 400)
+static K_MUTEX_DEFINE(conn_mutex);
+
static struct bt_conn *current_conn = NULL;
static struct bt_conn *next_conn = NULL;
static bool bonded_connection = false;
@@ -81,10 +83,6 @@ void connected(struct bt_conn *conn, uint8_t err) {
show_params(conn);
- connection_update_params();
-
- ble_reconfigure_tx_power();
-
// Prefer 2M both directions; 0 options = no specific constraints
// const struct bt_conn_le_phy_param phy_2m = {
// .options = 0,
@@ -110,6 +108,14 @@ void connected(struct bt_conn *conn, uint8_t err) {
} else {
current_conn = bt_conn_ref(conn);
}
+ k_mutex_lock(&conn_mutex, K_FOREVER);
+
+ connection_update_params();
+
+ k_mutex_unlock(&conn_mutex);
+
+ ble_reconfigure_tx_power();
+
advertising_stop();
ble_management_send_status_event();
@@ -183,28 +189,42 @@ void connection_disconnect(void) {
struct bt_conn *connection_get_current(void) { return current_conn; }
void connection_suspend(void) {
+ k_mutex_lock(&conn_mutex, K_FOREVER);
struct bt_conn *conn = connection_get_current();
if (conn != NULL) {
const struct bt_le_conn_param *param = PPCP_SUSPEND;
bt_conn_le_param_update(conn, param);
}
+
+ k_mutex_unlock(&conn_mutex);
}
-void connection_resume(void) { connection_update_params(); }
+void connection_resume(void) {
+ k_mutex_lock(&conn_mutex, K_FOREVER);
+ connection_update_params();
+ k_mutex_unlock(&conn_mutex);
+}
bool connection_is_bonded(void) { return bonded_connection; }
bool connection_is_high_speed(void) { return high_speed_requested; }
void connection_set_high_speed(void) {
+ k_mutex_lock(&conn_mutex, K_FOREVER);
+
high_speed_requested = true;
connection_update_params();
+ k_mutex_unlock(&conn_mutex);
}
void connection_set_low_speed(void) {
+ k_mutex_lock(&conn_mutex, K_FOREVER);
+
high_speed_requested = false;
connection_update_params();
+
+ k_mutex_unlock(&conn_mutex);
}
Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.