mirror of
https://github.com/mikecarper/MeshCore.git
synced 2026-10-23 21:33:53 +00:00
Fix Bluefruit role dispatch after disconnect frees connection
This commit is contained in:
1 parent
f7b5c40c3b
commit
0e70110640
8 files changed
+629
-1
No files matched your search
@@ -779,6 +779,7 @@ jobs:
|
||||
pio pkg install --global --tool 'platformio/toolchain-gccarmnoneeabi@1.140201.0'
|
||||
python3 -B test/test_nrf52_svc_compat.py -v
|
||||
python3 -B test/test_nrf52_ble_cccd_fix.py -v
|
||||
python3 -B test/test_nrf52_ble_disconnect.py -v
|
||||
pip install unicorn==2.1.4
|
||||
python3 -B test/test_nrf52_ble_dfu_fix.py -v
|
||||
|
||||
|
||||
@@ -10,6 +10,8 @@ Patch a private build copy; never edit PlatformIO's shared framework cache.
|
||||
With the pinned GCC 14/LTO toolchain, the optimized Bluefruit.begin() fails
|
||||
on the RAK4631 repeater: S140 rejects a valid single-peripheral role request
|
||||
with NRF_ERROR_RESOURCES. The unoptimized method starts DFU successfully.
|
||||
Cache the dispatch role before a disconnect deletes its BLEConnection. The
|
||||
remaining peripheral/central routing must not dereference that freed object.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
@@ -49,6 +51,32 @@ FIXED_BEGIN = (
|
||||
'AdafruitBluefruit::begin(uint8_t prph_count, uint8_t central_count)'
|
||||
)
|
||||
|
||||
DISCONNECT_PATCHES = (
|
||||
(
|
||||
" BLEConnection* conn = this->Connection(conn_hdl);\n\n"
|
||||
' LOG_LV2("BLE",',
|
||||
" BLEConnection* conn = this->Connection(conn_hdl);\n"
|
||||
" // Keep role routing valid after disconnect deletes the connection.\n"
|
||||
" uint8_t conn_role = conn ? conn->getRole() : BLE_GAP_ROLE_INVALID;\n\n"
|
||||
' LOG_LV2("BLE",',
|
||||
),
|
||||
(
|
||||
" conn = _connection[conn_hdl];\n\n"
|
||||
" // Invoke connect callback\n",
|
||||
" conn = _connection[conn_hdl];\n"
|
||||
" conn_role = conn->getRole();\n\n"
|
||||
" // Invoke connect callback\n",
|
||||
),
|
||||
(
|
||||
" if ( (conn == NULL) || (conn->getRole() == BLE_GAP_ROLE_PERIPH) )\n",
|
||||
" if ( (conn_role == BLE_GAP_ROLE_INVALID) || (conn_role == BLE_GAP_ROLE_PERIPH) )\n",
|
||||
),
|
||||
(
|
||||
" if ( (conn == NULL) || (conn->getRole() == BLE_GAP_ROLE_CENTRAL) )\n",
|
||||
" if ( (conn_role == BLE_GAP_ROLE_INVALID) || (conn_role == BLE_GAP_ROLE_CENTRAL) )\n",
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def patched_source(source):
|
||||
source = source.replace("\r\n", "\n")
|
||||
@@ -76,6 +104,7 @@ def patched_connection_source(source):
|
||||
|
||||
def patched_bluefruit_source(source):
|
||||
source = source.replace("\r\n", "\n")
|
||||
source = patched_disconnect_source(source)
|
||||
if FIXED_BEGIN in source and OLD_BEGIN not in source:
|
||||
return source
|
||||
if source.count(OLD_BEGIN) != 1:
|
||||
@@ -85,6 +114,21 @@ def patched_bluefruit_source(source):
|
||||
return source.replace(OLD_BEGIN, FIXED_BEGIN)
|
||||
|
||||
|
||||
def patched_disconnect_source(source):
|
||||
source = source.replace("\r\n", "\n")
|
||||
counts = [(source.count(old), source.count(fixed))
|
||||
for old, fixed in DISCONNECT_PATCHES]
|
||||
if all(old == 0 and fixed == 1 for old, fixed in counts):
|
||||
return source
|
||||
if not all(old == 1 and fixed == 0 for old, fixed in counts):
|
||||
raise RuntimeError(
|
||||
"nRF52 BLE disconnect fix: unrecognized framework dispatcher; review before building"
|
||||
)
|
||||
for old, fixed in DISCONNECT_PATCHES:
|
||||
source = source.replace(old, fixed)
|
||||
return source
|
||||
|
||||
|
||||
def patched_trace_source(source):
|
||||
"""Bracket real BLE dispatch in diagnostic builds, including stuck handlers."""
|
||||
include = '#include "bluefruit.h"\n'
|
||||
|
||||
+12
@@ -0,0 +1,12 @@
|
||||
The dispatcher in `bluefruit_dispatch.cpp` is copied without changes from
|
||||
`libraries/Bluefruit52Lib/src/bluefruit.cpp` at the framework revision pinned
|
||||
by `platformio.ini`:
|
||||
|
||||
https://github.com/meshcore-dev/Adafruit_nRF52_Arduino/blob/d541301665b40959682252911e57b11df3ee651a/libraries/Bluefruit52Lib/src/bluefruit.cpp
|
||||
|
||||
Its original BSD license is retained. `bluefruit.h` supplies host-side test
|
||||
doubles; `test.cpp` exercises the complete dispatcher after the production
|
||||
middleware transforms it. The unpatched dispatcher must fail under ASan with
|
||||
a heap-use-after-free on disconnect. The fixed dispatcher must retain role
|
||||
filtering, callback deferral, handle reuse, deletion order, and generic-event
|
||||
routing for peripheral and central connections.
|
||||
+138
@@ -0,0 +1,138 @@
|
||||
#pragma once
|
||||
|
||||
#include <cassert>
|
||||
#include <cstddef>
|
||||
#include <cstdint>
|
||||
#include <functional>
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
#define LOG_LV1(...) do {} while (0)
|
||||
#define LOG_LV2(...) do {} while (0)
|
||||
|
||||
constexpr uint8_t BLE_GAP_ROLE_INVALID = 0;
|
||||
constexpr uint8_t BLE_GAP_ROLE_PERIPH = 1;
|
||||
constexpr uint8_t BLE_GAP_ROLE_CENTRAL = 2;
|
||||
constexpr uint16_t BLE_CONN_HANDLE_INVALID = 0xffff;
|
||||
enum {
|
||||
BLE_GAP_EVT_CONNECTED = 1,
|
||||
BLE_GAP_EVT_DISCONNECTED,
|
||||
BLE_GAP_EVT_RSSI_CHANGED,
|
||||
BLE_EVT_USER_MEM_REQUEST,
|
||||
BLE_EVT_USER_MEM_RELEASE,
|
||||
BLE_TEST_GENERIC_EVENT,
|
||||
};
|
||||
|
||||
struct ble_gap_addr_t {
|
||||
uint8_t addr[6] = {}, addr_type = 0, addr_id_peer = 0;
|
||||
};
|
||||
struct ble_gap_conn_params_t {
|
||||
uint16_t max_conn_interval = 24, slave_latency = 4, conn_sup_timeout = 200;
|
||||
};
|
||||
struct ble_gap_evt_connected_t {
|
||||
ble_gap_addr_t peer_addr;
|
||||
ble_gap_conn_params_t conn_params;
|
||||
uint8_t role = BLE_GAP_ROLE_INVALID;
|
||||
};
|
||||
struct ble_gap_evt_disconnected_t { uint8_t reason = 0; };
|
||||
struct ble_gap_evt_rssi_changed_t { int8_t rssi = 0; };
|
||||
struct ble_evt_t {
|
||||
struct { uint16_t evt_id = 0; } header;
|
||||
struct {
|
||||
// The real SDK exposes the same handle storage through both views.
|
||||
union {
|
||||
struct { uint16_t conn_handle; } common_evt;
|
||||
struct {
|
||||
uint16_t conn_handle;
|
||||
struct {
|
||||
ble_gap_evt_connected_t connected;
|
||||
ble_gap_evt_disconnected_t disconnected;
|
||||
ble_gap_evt_rssi_changed_t rssi_changed;
|
||||
} params;
|
||||
} gap_evt;
|
||||
};
|
||||
} evt = {};
|
||||
};
|
||||
|
||||
inline std::vector<std::string> steps;
|
||||
inline std::vector<std::function<void()>> deferred;
|
||||
inline unsigned deleted_connections = 0;
|
||||
inline unsigned live_connections = 0;
|
||||
|
||||
template<typename Function, typename... Args>
|
||||
bool ada_callback(void*, size_t, Function function, Args... args) {
|
||||
steps.push_back("defer");
|
||||
deferred.emplace_back([=] { function(args...); });
|
||||
return true;
|
||||
}
|
||||
|
||||
inline void sd_ble_user_mem_reply(uint16_t, void*) {
|
||||
steps.push_back("memory_reply");
|
||||
}
|
||||
|
||||
struct BLEConnection {
|
||||
uint8_t _role;
|
||||
bool _connected = true;
|
||||
BLEConnection(uint16_t, const ble_gap_evt_connected_t* connected,
|
||||
uint8_t, uint8_t) : _role(connected->role) {
|
||||
++live_connections;
|
||||
steps.push_back("create");
|
||||
}
|
||||
~BLEConnection() {
|
||||
--live_connections;
|
||||
++deleted_connections;
|
||||
steps.push_back("delete");
|
||||
}
|
||||
// Read object storage so ASan checks the original dispatcher's freed access.
|
||||
__attribute__((noinline)) uint8_t getRole() { return _role; }
|
||||
void _eventHandler(ble_evt_t* event) {
|
||||
steps.push_back("connection");
|
||||
if (event->header.evt_id == BLE_GAP_EVT_DISCONNECTED) _connected = false;
|
||||
}
|
||||
};
|
||||
|
||||
using ConnectCallback = void (*)(uint16_t);
|
||||
using DisconnectCallback = void (*)(uint16_t, uint8_t);
|
||||
struct RoleHandler {
|
||||
const char* name;
|
||||
ConnectCallback _connect_cb = nullptr;
|
||||
DisconnectCallback _disconnect_cb = nullptr;
|
||||
void _eventHandler(ble_evt_t*) { steps.push_back(name); }
|
||||
};
|
||||
struct Handler {
|
||||
const char* name;
|
||||
void _eventHandler(ble_evt_t*) { steps.push_back(name); }
|
||||
};
|
||||
struct DiscoveryHandler : Handler {
|
||||
bool enabled = true;
|
||||
bool begun() const { return enabled; }
|
||||
};
|
||||
|
||||
class AdafruitBluefruit {
|
||||
public:
|
||||
BLEConnection* _connection[4] = {};
|
||||
uint16_t _conn_hdl = BLE_CONN_HANDLE_INVALID;
|
||||
struct {
|
||||
struct { uint8_t hvn_qsize = 3, wrcmd_qsize = 1; } prph, central;
|
||||
} _sd_cfg;
|
||||
RoleHandler Periph{"peripheral"}, Central{"central"};
|
||||
Handler Security{"security"}, Advertising{"advertising"}, Scanner{"scanner"},
|
||||
Gatt{"gatt"};
|
||||
DiscoveryHandler Discovery{{"discovery"}};
|
||||
void (*_event_cb)(ble_evt_t*) = nullptr;
|
||||
void (*_rssi_cb)(uint16_t, int8_t) = nullptr;
|
||||
|
||||
BLEConnection* Connection(uint16_t handle) {
|
||||
return handle < 4 ? _connection[handle] : nullptr;
|
||||
}
|
||||
unsigned connected() const {
|
||||
unsigned count = 0;
|
||||
for (auto* conn : _connection) count += conn && conn->_connected;
|
||||
return count;
|
||||
}
|
||||
void _stopConnLed() { steps.push_back("led_stop"); }
|
||||
void _setConnLed(bool enabled) {
|
||||
steps.push_back(enabled ? "led_on" : "led_off");
|
||||
}
|
||||
void _ble_handler(ble_evt_t*);
|
||||
};
|
||||
@@ -0,0 +1,197 @@
|
||||
/**************************************************************************/
|
||||
/*!
|
||||
@file bluefruit.cpp
|
||||
@author hathach (tinyusb.org)
|
||||
|
||||
@section LICENSE
|
||||
|
||||
Software License Agreement (BSD License)
|
||||
|
||||
Copyright (c) 2018, Adafruit Industries (adafruit.com)
|
||||
All rights reserved.
|
||||
|
||||
Redistribution and use in source and binary forms, with or without
|
||||
modification, are permitted provided that the following conditions are met:
|
||||
1. Redistributions of source code must retain the above copyright
|
||||
notice, this list of conditions and the following disclaimer.
|
||||
2. Redistributions in binary form must reproduce the above copyright
|
||||
notice, this list of conditions and the following disclaimer in the
|
||||
documentation and/or other materials provided with the distribution.
|
||||
3. Neither the name of the copyright holders nor the
|
||||
names of its contributors may be used to endorse or promote products
|
||||
derived from this software without specific prior written permission.
|
||||
|
||||
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS ''AS IS'' AND ANY
|
||||
EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
|
||||
WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
|
||||
DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER BE LIABLE FOR ANY
|
||||
DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
|
||||
(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
|
||||
LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
|
||||
ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
|
||||
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
|
||||
SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||
*/
|
||||
/**************************************************************************/
|
||||
|
||||
#include "bluefruit.h"
|
||||
|
||||
void AdafruitBluefruit::_ble_handler(ble_evt_t* evt)
|
||||
{
|
||||
// conn handle has fixed offset for all events
|
||||
uint16_t const conn_hdl = evt->evt.common_evt.conn_handle;
|
||||
BLEConnection* conn = this->Connection(conn_hdl);
|
||||
|
||||
LOG_LV2("BLE", "%s : Conn Handle = %d", dbg_ble_event_str(evt->header.evt_id), conn_hdl);
|
||||
|
||||
// GAP handler
|
||||
if ( conn )
|
||||
{
|
||||
conn->_eventHandler(evt);
|
||||
Security._eventHandler(evt);
|
||||
}
|
||||
|
||||
switch(evt->header.evt_id)
|
||||
{
|
||||
case BLE_GAP_EVT_CONNECTED:
|
||||
{
|
||||
// Turn on Conn LED
|
||||
_stopConnLed();
|
||||
_setConnLed(true);
|
||||
|
||||
ble_gap_evt_connected_t const * para = &evt->evt.gap_evt.params.connected;
|
||||
ble_gap_addr_t const* peer_addr = ¶->peer_addr;
|
||||
|
||||
(void) peer_addr;
|
||||
|
||||
LOG_LV2("GAP", "MAC = %02X:%02X:%02X:%02X:%02X:%02X, Type = %d, Resolved = %d",
|
||||
peer_addr->addr[5], peer_addr->addr[4], peer_addr->addr[3], peer_addr->addr[2], peer_addr->addr[1], peer_addr->addr[0],
|
||||
peer_addr->addr_type, peer_addr->addr_id_peer);
|
||||
LOG_LV2("GAP", "Conn Interval = %.2f ms, Latency = %d, Supervisor Timeout = %d ms",
|
||||
para->conn_params.max_conn_interval*1.25f, para->conn_params.slave_latency, 10*para->conn_params.conn_sup_timeout);
|
||||
|
||||
if ( _connection[conn_hdl] )
|
||||
{
|
||||
LOG_LV1("GAP", "Connection is already in used, something wrong !!");
|
||||
delete _connection[conn_hdl];
|
||||
_connection[conn_hdl] = NULL;
|
||||
}
|
||||
|
||||
// Transmission buffer pool
|
||||
uint8_t const hvn_qsize = (para->role == BLE_GAP_ROLE_PERIPH) ? _sd_cfg.prph.hvn_qsize : _sd_cfg.central.hvn_qsize;
|
||||
uint8_t const wrcmd_qsize = (para->role == BLE_GAP_ROLE_PERIPH) ? _sd_cfg.prph.wrcmd_qsize : _sd_cfg.central.wrcmd_qsize;
|
||||
|
||||
_connection[conn_hdl] = new BLEConnection(conn_hdl, para, hvn_qsize, wrcmd_qsize);
|
||||
conn = _connection[conn_hdl];
|
||||
|
||||
// Invoke connect callback
|
||||
if ( conn->getRole() == BLE_GAP_ROLE_PERIPH )
|
||||
{
|
||||
if ( Periph._connect_cb ) ada_callback(NULL, 0, Periph._connect_cb, conn_hdl);
|
||||
}else
|
||||
{
|
||||
if ( Central._connect_cb ) ada_callback(NULL, 0, Central._connect_cb, conn_hdl);
|
||||
}
|
||||
}
|
||||
break;
|
||||
|
||||
case BLE_GAP_EVT_DISCONNECTED:
|
||||
{
|
||||
ble_gap_evt_disconnected_t const* para = &evt->evt.gap_evt.params.disconnected;
|
||||
|
||||
LOG_LV2("GAP", "Disconnect Reason: %s", dbg_hci_str(evt->evt.gap_evt.params.disconnected.reason));
|
||||
|
||||
// Turn off Conn LED If not connected at all
|
||||
if ( !this->connected() ) _setConnLed(false);
|
||||
|
||||
// Invoke disconnect callback
|
||||
if ( conn->getRole() == BLE_GAP_ROLE_PERIPH )
|
||||
{
|
||||
if ( Periph._disconnect_cb ) ada_callback(NULL, 0, Periph._disconnect_cb, conn_hdl, para->reason);
|
||||
}else
|
||||
{
|
||||
if ( Central._disconnect_cb ) ada_callback(NULL, 0, Central._disconnect_cb, conn_hdl, para->reason);
|
||||
}
|
||||
|
||||
delete _connection[conn_hdl];
|
||||
_connection[conn_hdl] = NULL;
|
||||
}
|
||||
break;
|
||||
|
||||
case BLE_GAP_EVT_RSSI_CHANGED:
|
||||
{
|
||||
ble_gap_evt_rssi_changed_t const * rssi_changed = &evt->evt.gap_evt.params.rssi_changed;
|
||||
if ( _rssi_cb )
|
||||
{
|
||||
ada_callback(NULL, 0, _rssi_cb, conn_hdl, rssi_changed->rssi);
|
||||
}
|
||||
}
|
||||
break;
|
||||
|
||||
case BLE_EVT_USER_MEM_REQUEST:
|
||||
// We will handle Long Write sequence (RW Authorize PREP_WRITE_REQ)
|
||||
sd_ble_user_mem_reply(conn_hdl, NULL);
|
||||
break;
|
||||
|
||||
case BLE_EVT_USER_MEM_RELEASE:
|
||||
// nothing to do
|
||||
break;
|
||||
|
||||
default: break;
|
||||
}
|
||||
|
||||
Advertising._eventHandler(evt);
|
||||
Scanner._eventHandler(evt);
|
||||
|
||||
/*------------- BLE Peripheral Events -------------*/
|
||||
/* Only handle Peripheral events with matched connection handle
|
||||
* or a few special one
|
||||
* - Connected event
|
||||
* - Advertising timeout (could be connected and advertising at the same time)
|
||||
*/
|
||||
if ( conn_hdl == _conn_hdl ||
|
||||
evt->header.evt_id == BLE_GAP_EVT_CONNECTED)
|
||||
{
|
||||
switch ( evt->header.evt_id )
|
||||
{
|
||||
case BLE_GAP_EVT_CONNECTED:
|
||||
{
|
||||
ble_gap_evt_connected_t* para = &evt->evt.gap_evt.params.connected;
|
||||
|
||||
if (para->role == BLE_GAP_ROLE_PERIPH)
|
||||
{
|
||||
_conn_hdl = evt->evt.gap_evt.conn_handle;
|
||||
}
|
||||
}
|
||||
break;
|
||||
|
||||
case BLE_GAP_EVT_DISCONNECTED:
|
||||
_conn_hdl = BLE_CONN_HANDLE_INVALID;
|
||||
break;
|
||||
|
||||
default: break;
|
||||
}
|
||||
}
|
||||
|
||||
// Periph event handler (also handle generic non-connection event)
|
||||
if ( (conn == NULL) || (conn->getRole() == BLE_GAP_ROLE_PERIPH) )
|
||||
{
|
||||
Periph._eventHandler(evt);
|
||||
}
|
||||
|
||||
// Central Event Handler (also handle generic non-connection event)
|
||||
if ( (conn == NULL) || (conn->getRole() == BLE_GAP_ROLE_CENTRAL) )
|
||||
{
|
||||
Central._eventHandler(evt);
|
||||
}
|
||||
|
||||
// Discovery Event Handler
|
||||
if ( Discovery.begun() ) Discovery._eventHandler(evt);
|
||||
|
||||
// GATTs characteristics event handler
|
||||
Gatt._eventHandler(evt);
|
||||
|
||||
// User callback if set
|
||||
if (_event_cb) _event_cb(evt);
|
||||
}
|
||||
|
||||
+151
@@ -0,0 +1,151 @@
|
||||
#include "bluefruit.h"
|
||||
|
||||
#include <algorithm>
|
||||
|
||||
static unsigned peripheral_connects = 0, central_connects = 0;
|
||||
static unsigned peripheral_disconnects = 0, central_disconnects = 0;
|
||||
static uint16_t callback_handle = BLE_CONN_HANDLE_INVALID;
|
||||
static uint8_t callback_reason = 0;
|
||||
static AdafruitBluefruit* active = nullptr;
|
||||
|
||||
static void peripheralConnect(uint16_t handle) {
|
||||
++peripheral_connects;
|
||||
callback_handle = handle;
|
||||
}
|
||||
static void centralConnect(uint16_t handle) {
|
||||
++central_connects;
|
||||
callback_handle = handle;
|
||||
}
|
||||
static void peripheralDisconnect(uint16_t handle, uint8_t reason) {
|
||||
assert(active->Connection(handle) == nullptr);
|
||||
++peripheral_disconnects;
|
||||
callback_handle = handle;
|
||||
callback_reason = reason;
|
||||
}
|
||||
static void centralDisconnect(uint16_t handle, uint8_t reason) {
|
||||
assert(active->Connection(handle) == nullptr);
|
||||
++central_disconnects;
|
||||
callback_handle = handle;
|
||||
callback_reason = reason;
|
||||
}
|
||||
static void userEvent(ble_evt_t* event) {
|
||||
if (event->header.evt_id == BLE_GAP_EVT_DISCONNECTED) {
|
||||
assert(active->Connection(event->evt.common_evt.conn_handle) == nullptr);
|
||||
}
|
||||
steps.push_back("user");
|
||||
}
|
||||
static ble_evt_t event(uint16_t id, uint16_t handle) {
|
||||
ble_evt_t result = {};
|
||||
result.header.evt_id = id;
|
||||
result.evt.common_evt.conn_handle = handle;
|
||||
return result;
|
||||
}
|
||||
static void runDeferred() {
|
||||
for (const auto& callback : deferred) callback();
|
||||
deferred.clear();
|
||||
}
|
||||
static void checkSteps(std::initializer_list<const char*> expected) {
|
||||
assert(steps.size() == expected.size());
|
||||
assert(std::equal(steps.begin(), steps.end(), expected.begin()));
|
||||
steps.clear();
|
||||
}
|
||||
|
||||
static void connect(AdafruitBluefruit& bluefruit, uint8_t role, uint16_t handle) {
|
||||
auto connected = event(BLE_GAP_EVT_CONNECTED, handle);
|
||||
connected.evt.gap_evt.params.connected.role = role;
|
||||
bluefruit._ble_handler(&connected);
|
||||
assert(live_connections == 1);
|
||||
assert(bluefruit.Connection(handle)->getRole() == role);
|
||||
assert(bluefruit._conn_hdl == (role == BLE_GAP_ROLE_PERIPH
|
||||
? handle : BLE_CONN_HANDLE_INVALID));
|
||||
assert(deferred.size() == 1);
|
||||
assert(steps[0] == "led_stop");
|
||||
assert(steps[1] == "led_on");
|
||||
assert(steps[2] == "create");
|
||||
assert(steps[3] == "defer");
|
||||
assert(steps[4] == "advertising");
|
||||
assert(steps[5] == "scanner");
|
||||
assert(steps[6] == (role == BLE_GAP_ROLE_PERIPH ? "peripheral" : "central"));
|
||||
assert(steps[7] == "discovery" && steps[8] == "gatt" && steps[9] == "user");
|
||||
assert(steps.size() == 10);
|
||||
steps.clear();
|
||||
runDeferred();
|
||||
assert(callback_handle == handle);
|
||||
}
|
||||
|
||||
static void disconnect(AdafruitBluefruit& bluefruit, uint8_t role,
|
||||
uint16_t handle, uint8_t reason) {
|
||||
const unsigned old_deleted = deleted_connections;
|
||||
const unsigned old_peripheral = peripheral_disconnects;
|
||||
const unsigned old_central = central_disconnects;
|
||||
auto disconnected = event(BLE_GAP_EVT_DISCONNECTED, handle);
|
||||
disconnected.evt.gap_evt.params.disconnected.reason = reason;
|
||||
bluefruit._ble_handler(&disconnected);
|
||||
assert(live_connections == 0 && deleted_connections == old_deleted + 1);
|
||||
assert(bluefruit.Connection(handle) == nullptr);
|
||||
assert(bluefruit._conn_hdl == BLE_CONN_HANDLE_INVALID);
|
||||
assert(deferred.size() == 1);
|
||||
assert(peripheral_disconnects == old_peripheral);
|
||||
assert(central_disconnects == old_central);
|
||||
if (role == BLE_GAP_ROLE_PERIPH) {
|
||||
checkSteps({"connection", "security", "led_off", "defer", "delete",
|
||||
"advertising", "scanner", "peripheral", "discovery", "gatt", "user"});
|
||||
} else {
|
||||
checkSteps({"connection", "security", "led_off", "defer", "delete",
|
||||
"advertising", "scanner", "central", "discovery", "gatt", "user"});
|
||||
}
|
||||
runDeferred();
|
||||
assert(callback_handle == handle && callback_reason == reason);
|
||||
assert(peripheral_disconnects == old_peripheral + (role == BLE_GAP_ROLE_PERIPH));
|
||||
assert(central_disconnects == old_central + (role == BLE_GAP_ROLE_CENTRAL));
|
||||
}
|
||||
|
||||
int main() {
|
||||
AdafruitBluefruit bluefruit;
|
||||
active = &bluefruit;
|
||||
bluefruit.Periph._connect_cb = peripheralConnect;
|
||||
bluefruit.Central._connect_cb = centralConnect;
|
||||
bluefruit.Periph._disconnect_cb = peripheralDisconnect;
|
||||
bluefruit.Central._disconnect_cb = centralDisconnect;
|
||||
bluefruit._event_cb = userEvent;
|
||||
|
||||
// Exercise peripheral and central roles, handle reuse, and both local and
|
||||
// supervision-timeout disconnect reasons without relying on real hardware.
|
||||
for (unsigned i = 0; i < 20; ++i) {
|
||||
const uint8_t role = (i & 1) ? BLE_GAP_ROLE_CENTRAL : BLE_GAP_ROLE_PERIPH;
|
||||
const uint16_t handle = i % 4;
|
||||
connect(bluefruit, role, handle);
|
||||
auto ordinary = event(BLE_TEST_GENERIC_EVENT, handle);
|
||||
bluefruit._ble_handler(&ordinary);
|
||||
if (role == BLE_GAP_ROLE_PERIPH) {
|
||||
checkSteps({"connection", "security", "advertising", "scanner", "peripheral",
|
||||
"discovery", "gatt", "user"});
|
||||
} else {
|
||||
checkSteps({"connection", "security", "advertising", "scanner", "central",
|
||||
"discovery", "gatt", "user"});
|
||||
}
|
||||
disconnect(bluefruit, role, handle, (i & 2) ? 0x08 : 0x16);
|
||||
}
|
||||
assert(peripheral_connects == 10 && central_connects == 10);
|
||||
assert(peripheral_disconnects == 10 && central_disconnects == 10);
|
||||
|
||||
// Unknown-handle generic events still reach both role handlers.
|
||||
auto generic = event(BLE_EVT_USER_MEM_REQUEST, BLE_CONN_HANDLE_INVALID);
|
||||
bluefruit._ble_handler(&generic);
|
||||
checkSteps({"memory_reply", "advertising", "scanner", "peripheral", "central",
|
||||
"discovery", "gatt", "user"});
|
||||
|
||||
// CONNECTED replaces an already occupied handle in the pinned framework.
|
||||
// The routing snapshot must be refreshed for the newly allocated role.
|
||||
connect(bluefruit, BLE_GAP_ROLE_PERIPH, 2);
|
||||
const unsigned old_deleted = deleted_connections;
|
||||
auto replacement = event(BLE_GAP_EVT_CONNECTED, 2);
|
||||
replacement.evt.gap_evt.params.connected.role = BLE_GAP_ROLE_CENTRAL;
|
||||
bluefruit._ble_handler(&replacement);
|
||||
assert(deleted_connections == old_deleted + 1 && live_connections == 1);
|
||||
checkSteps({"connection", "security", "led_stop", "led_on", "delete", "create",
|
||||
"defer", "advertising", "scanner", "central", "discovery", "gatt", "user"});
|
||||
runDeferred();
|
||||
disconnect(bluefruit, BLE_GAP_ROLE_CENTRAL, 2, 0x13);
|
||||
assert(deleted_connections == 22 && live_connections == 0);
|
||||
}
|
||||
@@ -55,8 +55,9 @@ class Nrf52BleCccdFixTest(unittest.TestCase):
|
||||
self.assertNotIn("mesh_nrf52_service_cccd_save", app)
|
||||
|
||||
def test_bluefruit_begin_uses_private_unoptimized_copy(self):
|
||||
dispatcher = (ROOT / "test/fixtures/nrf52_ble_disconnect/bluefruit_dispatch.cpp").read_text()
|
||||
patched = module.patched_bluefruit_source(
|
||||
"before\n" + module.OLD_BEGIN + "\nafter\n"
|
||||
"before\n" + module.OLD_BEGIN + "\nafter\n" + dispatcher
|
||||
)
|
||||
self.assertEqual(patched.count(module.FIXED_BEGIN), 1)
|
||||
self.assertEqual(module.patched_bluefruit_source(patched), patched)
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
"""Exercise the production Bluefruit dispatcher fix under ASan and UBSan."""
|
||||
|
||||
from importlib.util import module_from_spec, spec_from_file_location
|
||||
import os
|
||||
from pathlib import Path
|
||||
import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
FIXTURE = ROOT / "test/fixtures/nrf52_ble_disconnect"
|
||||
spec = spec_from_file_location("nrf52_ble_patcher", ROOT / "scripts/nrf52_ble_cccd_fix.py")
|
||||
patcher = module_from_spec(spec)
|
||||
spec.loader.exec_module(patcher)
|
||||
|
||||
|
||||
class Nrf52BleDisconnectTest(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.source = (FIXTURE / "bluefruit_dispatch.cpp").read_text()
|
||||
|
||||
def test_patch_is_idempotent_and_preserves_newlines(self):
|
||||
fixed = patcher.patched_disconnect_source(self.source)
|
||||
self.assertNotEqual(fixed, self.source)
|
||||
self.assertEqual(patcher.patched_disconnect_source(fixed), fixed)
|
||||
self.assertEqual(patcher.patched_disconnect_source(
|
||||
self.source.replace("\n", "\r\n")), fixed)
|
||||
|
||||
def test_changed_duplicate_or_partial_dispatchers_fail_closed(self):
|
||||
for old, fixed in patcher.DISCONNECT_PATCHES:
|
||||
for malformed in (
|
||||
self.source.replace(old, "changed framework source"),
|
||||
self.source + old,
|
||||
self.source.replace(old, fixed)):
|
||||
with self.subTest(marker=old, source=malformed[:30]):
|
||||
with self.assertRaises(RuntimeError):
|
||||
patcher.patched_disconnect_source(malformed)
|
||||
fixed_source = patcher.patched_disconnect_source(self.source)
|
||||
for _, fixed in patcher.DISCONNECT_PATCHES:
|
||||
with self.subTest(fixed_marker=fixed):
|
||||
with self.assertRaises(RuntimeError):
|
||||
patcher.patched_disconnect_source(fixed_source + fixed)
|
||||
|
||||
def test_private_bluefruit_pipeline_applies_startup_and_disconnect_fixes(self):
|
||||
source = patcher.OLD_BEGIN + " { return true; }\n" + self.source
|
||||
fixed = patcher.patched_bluefruit_source(source)
|
||||
self.assertIn(patcher.FIXED_BEGIN, fixed)
|
||||
self.assertEqual(patcher.patched_bluefruit_source(fixed), fixed)
|
||||
for old, new in patcher.DISCONNECT_PATCHES:
|
||||
self.assertNotIn(old, fixed)
|
||||
self.assertEqual(fixed.count(new), 1)
|
||||
|
||||
def run_dispatcher(self, source):
|
||||
with tempfile.TemporaryDirectory(prefix="nrf52-ble-disconnect-") as directory:
|
||||
work = Path(directory)
|
||||
dispatcher = work / "dispatch.cpp"
|
||||
dispatcher.write_text(source)
|
||||
binary = work / "dispatch"
|
||||
subprocess.run([
|
||||
"c++", "-std=c++17", "-O1", "-g", "-Wall", "-Wextra", "-Werror",
|
||||
"-fsanitize=address,undefined", "-fno-omit-frame-pointer",
|
||||
"-fno-pie", "-no-pie", "-I" + str(FIXTURE),
|
||||
str(dispatcher), str(FIXTURE / "test.cpp"), "-o", str(binary),
|
||||
], check=True, capture_output=True, text=True)
|
||||
environment = os.environ.copy()
|
||||
environment["ASAN_OPTIONS"] = "detect_leaks=1:halt_on_error=1"
|
||||
environment["UBSAN_OPTIONS"] = "halt_on_error=1:print_stacktrace=1"
|
||||
return subprocess.run([str(binary)], capture_output=True,
|
||||
text=True, env=environment)
|
||||
|
||||
def test_original_dispatcher_reproduces_heap_use_after_free(self):
|
||||
result = self.run_dispatcher(self.source)
|
||||
self.assertNotEqual(result.returncode, 0)
|
||||
self.assertIn("AddressSanitizer: heap-use-after-free", result.stderr)
|
||||
self.assertIn("BLEConnection::getRole()", result.stderr)
|
||||
|
||||
def test_fixed_dispatcher_preserves_both_roles_and_reconnects(self):
|
||||
fixed = patcher.patched_disconnect_source(self.source)
|
||||
result = self.run_dispatcher(fixed)
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user