Skip to content

Commit 3c931ec

Browse files
committed
services/bluetooth: Defer bonding cbs to KernelMain to avoid deadlock
- schedule BLE bonding change notifications on the launcher task when invoked from NimBLE - queue bonding creation finalization to KernelMain so bt_lock is taken outside the host stack Signed-off-by: Joshua Jun <joshuajun@proton.me>
1 parent cc2f4d1 commit 3c931ec

2 files changed

Lines changed: 79 additions & 20 deletions

File tree

src/fw/services/common/bluetooth/bonding.c

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,46 @@
2020

2121
#include "services/common/bluetooth/bluetooth_persistent_storage.h"
2222
#include "services/common/bluetooth/local_addr.h"
23+
#include "kernel/event_loop.h"
24+
#include "kernel/pbl_malloc.h"
2325
#include "system/logging.h"
2426

2527
#include <bluetooth/bonding_sync.h>
2628
#include <bluetooth/bluetooth_types.h>
2729
#include <bluetooth/sm_types.h>
2830

31+
typedef struct {
32+
BTBondingID bonding_id;
33+
BTDeviceAddress addr;
34+
bool is_gateway;
35+
} CreateBondingContext;
36+
37+
static void prv_finalize_create_bonding(BTBondingID bonding_id,
38+
const BTDeviceAddress *addr,
39+
bool is_gateway) {
40+
bt_lock();
41+
GAPLEConnection *connection = gap_le_connection_by_addr(addr);
42+
if (connection) {
43+
connection->bonding_id = bonding_id;
44+
connection->is_gateway = is_gateway;
45+
46+
if (!connection->is_gateway) {
47+
PBL_LOG(LOG_LEVEL_DEBUG, "New bonding is not gateway?");
48+
}
49+
50+
gap_le_device_name_request(&connection->device);
51+
} else {
52+
PBL_LOG(LOG_LEVEL_ERROR, "Couldn't find connection for bonding!");
53+
}
54+
bt_unlock();
55+
}
56+
57+
static void prv_finalize_create_bonding_cb(void *data) {
58+
CreateBondingContext *context = data;
59+
prv_finalize_create_bonding(context->bonding_id, &context->addr, context->is_gateway);
60+
kernel_free(context);
61+
}
62+
2963
void bt_driver_cb_handle_create_bonding(const BleBonding *bonding,
3064
const BTDeviceAddress *addr) {
3165
#if !defined(PLATFORM_TINTIN)
@@ -44,24 +78,21 @@ void bt_driver_cb_handle_create_bonding(const BleBonding *bonding,
4478
bonding->is_gateway, NULL,
4579
should_pin_address,
4680
flags);
47-
bt_lock();
48-
{
49-
GAPLEConnection *connection = gap_le_connection_by_addr(addr);
50-
if (connection) {
51-
// Associate the connection with the bonding:
52-
connection->bonding_id = bonding_id;
53-
connection->is_gateway = bonding->is_gateway;
54-
55-
if (!connection->is_gateway) {
56-
PBL_LOG(LOG_LEVEL_DEBUG, "New bonding is not gateway?");
57-
}
81+
if (bonding_id == BT_BONDING_ID_INVALID) {
82+
return;
83+
}
5884

59-
// Request device name. iOS returns an "anonymized" device name before encryption, like
60-
// "iPhone" and only returns the real name i.e. "Martijn's iPhone" after encryption is set up.
61-
gap_le_device_name_request(&connection->device);
62-
} else {
63-
PBL_LOG(LOG_LEVEL_ERROR, "Couldn't find connection for bonding!");
64-
}
85+
if (launcher_task_is_current_task()) {
86+
prv_finalize_create_bonding(bonding_id, addr, bonding->is_gateway);
87+
return;
6588
}
66-
bt_unlock();
89+
90+
CreateBondingContext *context = kernel_malloc_check(sizeof(*context));
91+
92+
*context = (CreateBondingContext) {
93+
.bonding_id = bonding_id,
94+
.addr = *addr,
95+
.is_gateway = bonding->is_gateway,
96+
};
97+
launcher_task_add_callback(prv_finalize_create_bonding_cb, context);
6798
}

src/fw/services/normal/bluetooth/bluetooth_persistent_storage.c

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "comm/ble/kernel_le_client/kernel_le_client.h"
2626
#include "comm/bt_lock.h"
2727
#include "console/prompt.h"
28+
#include "kernel/event_loop.h"
2829
#include "kernel/pbl_malloc.h"
2930
#include "os/mutex.h"
3031
#include "services/common/analytics/analytics.h"
@@ -583,8 +584,13 @@ static unsigned int prv_get_num_pairings_by_type(BtPersistBondingType type) {
583584
///////////////////////////////////////////////////////////////////////////////////////////////////
584585
//! BLE Pairing Info
585586
void gap_le_connection_handle_bonding_change(BTBondingID bonding, BtPersistBondingOp op);
586-
static void prv_call_ble_bonding_change_handlers(BTBondingID bonding,
587-
BtPersistBondingOp op) {
587+
typedef struct {
588+
BTBondingID bonding;
589+
BtPersistBondingOp op;
590+
} BleBondingChangeContext;
591+
592+
static void prv_call_ble_bonding_change_handlers_impl(BTBondingID bonding,
593+
BtPersistBondingOp op) {
588594
prv_update_active_gateway_if_needed(bonding, op);
589595

590596
if (!bt_ctl_is_bluetooth_running()) {
@@ -597,6 +603,28 @@ static void prv_call_ble_bonding_change_handlers(BTBondingID bonding,
597603
prv_call_common_bonding_change_handlers(bonding, op);
598604
}
599605

606+
static void prv_call_ble_bonding_change_handlers_cb(void *data) {
607+
BleBondingChangeContext *context = data;
608+
prv_call_ble_bonding_change_handlers_impl(context->bonding, context->op);
609+
kernel_free(context);
610+
}
611+
612+
static void prv_call_ble_bonding_change_handlers(BTBondingID bonding,
613+
BtPersistBondingOp op) {
614+
if (launcher_task_is_current_task()) {
615+
prv_call_ble_bonding_change_handlers_impl(bonding, op);
616+
return;
617+
}
618+
619+
BleBondingChangeContext *context = kernel_malloc_check(sizeof(*context));
620+
621+
*context = (BleBondingChangeContext) {
622+
.bonding = bonding,
623+
.op = op,
624+
};
625+
launcher_task_add_callback(prv_call_ble_bonding_change_handlers_cb, context);
626+
}
627+
600628
typedef struct {
601629
SMPairingInfo pairing_info;
602630
BTBondingID key_out;

0 commit comments

Comments
 (0)