From 3ed1bc7e2077b666ae36c3c2f9581e458eeff0b0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 1 Oct 2026 13:50:46 -0500 Subject: [PATCH] [esp32_ble_tracker] Guard Bluedroid against stale queued connection requests (#19375) --- .../components/esp32_ble_tracker/__init__.py | 24 ++++++ .../esp32_ble_tracker/bluedroid_stubs.cpp | 48 ++++++++++++ esphome/core/defines.h | 1 + .../test_direct_conn_guard.py | 73 +++++++++++++++++++ 4 files changed, 146 insertions(+) create mode 100644 esphome/components/esp32_ble_tracker/bluedroid_stubs.cpp create mode 100644 tests/component_tests/esp32_ble_tracker/test_direct_conn_guard.py diff --git a/esphome/components/esp32_ble_tracker/__init__.py b/esphome/components/esp32_ble_tracker/__init__.py index 205acc55a5..fc52b92ee7 100644 --- a/esphome/components/esp32_ble_tracker/__init__.py +++ b/esphome/components/esp32_ble_tracker/__init__.py @@ -436,6 +436,25 @@ async def to_code(config: ConfigType) -> None: cg.add_define("USE_ESP32_BLE_SOFTWARE_COEXISTENCE") +# First tagged release per series with espressif/esp-idf@82e71c1767 (see bluedroid_stubs.cpp). +# A series without an entry keeps the guard until a fixed release is tagged; the guard is +# harmless on fixed sources. The 5.4, 5.5 and 6.1 branches carry the fix but have no tag yet. +DIRECT_CONN_FIX_VERSIONS = { + (5, 2): cv.Version(5, 2, 8), + (5, 3): cv.Version(5, 3, 6), + (6, 0): cv.Version(6, 0, 3), +} +DIRECT_CONN_FIX_ALL_FROM = cv.Version(6, 2, 0) + + +def _needs_direct_conn_guard() -> bool: + ver = idf_version() + if ver >= DIRECT_CONN_FIX_ALL_FROM: + return False + fixed = DIRECT_CONN_FIX_VERSIONS.get((ver.major, ver.minor)) + return fixed is None or ver < fixed + + # This needs to be run as a job with very low priority so that all components have # chance to call register_ble_tracker and register_client before the list is checked # and added to the global defines list. @@ -452,6 +471,11 @@ async def _add_ble_features() -> None: if BLEFeatures.ESP_BT_DEVICE in required_features: cg.add_define("USE_ESP32_BLE_DEVICE") cg.add_define("USE_ESP32_BLE_UUID") + if cg.get_slot_count(CLIENT_COUNT_DEFINE) and _needs_direct_conn_guard(): + # --undefined keeps the wrapper, libsrc.a is scanned before the IDF libraries + cg.add_define("USE_ESP32_BLE_TRACKER_DIRECT_CONN_GUARD") + cg.add_build_flag("-Wl,--wrap=l2cble_init_direct_conn") + cg.add_build_flag("-Wl,--undefined=__wrap_l2cble_init_direct_conn") ESP32_BLE_START_SCAN_ACTION_SCHEMA = cv.Schema( diff --git a/esphome/components/esp32_ble_tracker/bluedroid_stubs.cpp b/esphome/components/esp32_ble_tracker/bluedroid_stubs.cpp new file mode 100644 index 0000000000..5cedc22d06 --- /dev/null +++ b/esphome/components/esp32_ble_tracker/bluedroid_stubs.cpp @@ -0,0 +1,48 @@ +/* + * Bluedroid queues outgoing BLE connections as raw link block pointers and does + * not drop them when the block is released, so btm_send_pending_direct_conn() + * can start a connect on a released block and l2c_link_timeout() later crashes + * on its null timer parameter. Mirrors espressif/esp-idf@82e71c1767; codegen + * only enables it for releases without that commit. + */ + +#include "esphome/core/defines.h" + +#ifdef USE_ESP32_BLE_TRACKER_DIRECT_CONN_GUARD + +#include +#include +#include "esphome/core/log.h" + +namespace esphome::esp32_ble_tracker { +static const char *const TAG = "esp32_ble_tracker"; +} // namespace esphome::esp32_ble_tracker + +static_assert(ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(6, 2, 0), + "ESP-IDF 6.2 and later have the fix, this guard should not be enabled (esphome/esphome#19373)"); + +// NOLINTBEGIN(bugprone-reserved-identifier,cert-dcl37-c,cert-dcl51-cpp,readability-identifier-naming) +extern "C" { + +bool __real_l2cble_init_direct_conn(void *p_lcb); +void l2cu_release_lcb(void *p_lcb); + +bool __wrap_l2cble_init_direct_conn(void *p_lcb) { + // in_use is the first member of the private tL2C_LCB (checked ESP-IDF 5.0 to 6.1) + const auto *in_use = static_cast(p_lcb); + if (p_lcb == nullptr || *in_use == 0) { + ESP_LOGW(esphome::esp32_ble_tracker::TAG, "Dropped queued connect on a released link block"); + return false; + } + const bool started = __real_l2cble_init_direct_conn(p_lcb); + // Every failure path releases the block except unknown device, also fixed upstream + if (!started && *in_use != 0) { + l2cu_release_lcb(p_lcb); + } + return started; +} + +} // extern "C" +// NOLINTEND(bugprone-reserved-identifier,cert-dcl37-c,cert-dcl51-cpp,readability-identifier-naming) + +#endif // USE_ESP32_BLE_TRACKER_DIRECT_CONN_GUARD diff --git a/esphome/core/defines.h b/esphome/core/defines.h index 88f35dac46..9b3252fa05 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -389,6 +389,7 @@ #define USE_ESP32_BLE_SERVER_ON_CONNECT #define USE_ESP32_BLE_SERVER_ON_DISCONNECT #define USE_ESP32_BLE_TRACKER +#define USE_ESP32_BLE_TRACKER_DIRECT_CONN_GUARD #define USE_BLE_GATT_CLIENT #define ESPHOME_BLE_GATT_CLIENT_COUNT 1 #define ESPHOME_ESP32_BLE_TRACKER_LISTENER_COUNT 1 diff --git a/tests/component_tests/esp32_ble_tracker/test_direct_conn_guard.py b/tests/component_tests/esp32_ble_tracker/test_direct_conn_guard.py new file mode 100644 index 0000000000..d3aa2ba878 --- /dev/null +++ b/tests/component_tests/esp32_ble_tracker/test_direct_conn_guard.py @@ -0,0 +1,73 @@ +"""The Bluedroid queued connection guard is emitted only for client builds on unfixed ESP-IDF.""" + +from __future__ import annotations + +from collections.abc import Callable +from pathlib import Path + +import pytest + +from esphome import config_validation as cv +from esphome.components import esp32_ble_tracker +from esphome.core import CORE + +# Spelled out so a typo in the component's flags fails here +_GUARD_FLAGS = { + "-Wl,--wrap=l2cble_init_direct_conn", + "-Wl,--undefined=__wrap_l2cble_init_direct_conn", +} + + +def _pin_idf(monkeypatch: pytest.MonkeyPatch, idf: str) -> None: + monkeypatch.setattr(esp32_ble_tracker, "idf_version", lambda: cv.Version.parse(idf)) + + +@pytest.mark.parametrize( + ("config_file", "idf", "expected"), + [ + pytest.param("scan_window_raised.yaml", "5.5.5", True, id="client"), + pytest.param("scan_window_raised.yaml", "6.0.3", False, id="client_fixed_idf"), + pytest.param("scan_window_scan_only.yaml", "5.5.5", False, id="scan_only"), + ], +) +def test_guard_only_in_client_builds_on_unfixed_idf( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], + monkeypatch: pytest.MonkeyPatch, + config_file: str, + idf: str, + expected: bool, +) -> None: + _pin_idf(monkeypatch, idf) + generate_main(component_config_path(config_file)) + assert (CORE.build_flags >= _GUARD_FLAGS) is expected + assert CORE.build_flags.isdisjoint(_GUARD_FLAGS) is not expected + defines = {define.name for define in CORE.defines} + assert ("USE_ESP32_BLE_TRACKER_DIRECT_CONN_GUARD" in defines) is expected + + +@pytest.mark.parametrize( + ("idf", "expected"), + [ + ("5.1.6", True), # series that never got the fix + ("5.2.7", True), + ("5.2.8", False), + ("5.3.5", True), + ("5.3.6", False), + ("5.4.4", True), + ("5.4.5", True), # no fixed 5.4, 5.5 or 6.1 tag yet + ("5.5.5", True), + ("5.5.6", True), + ("6.0.2", True), + ("6.0.3", False), + ("6.1.0", True), + ("6.1.1", True), + ("6.2.0", False), + ("7.0.0", False), + ], +) +def test_needs_direct_conn_guard( + monkeypatch: pytest.MonkeyPatch, idf: str, expected: bool +) -> None: + _pin_idf(monkeypatch, idf) + assert esp32_ble_tracker._needs_direct_conn_guard() is expected