[esp32_ble_tracker] Guard Bluedroid against stale queued connection requests (#19375)

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(

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 <esp_idf_version.h>
+#include <cstdint>
+#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<const uint8_t *>(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

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

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