From 11b54cf92c934c527afc6d3b665701364037d468 Mon Sep 17 00:00:00 2001 From: Stefanos Anastasiou Date: Sun, 19 Jul 2026 16:01:18 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20whiteListRemove=20use-after-free=20?= =?UTF-8?q?=E2=80=94=20loop=20continues=20on=20invalidated=20iterator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: whiteListRemove use-after-free - stop iterating after erase + shrink whiteListRemove erases the matched element (invalidating the iterator) and shrink-to-fits m_whiteList (freeing the buffer the iterator points into), then continues the loop on the dangling iterator. A layout-dependent spurious re-match then calls erase() with a foreign iterator, and the vector's internal element move writes across unrelated heap memory. Observed on ESP32-C3 as intermittent heap corruption under whitelist churn (multi_heap_free bad-head asserts with address bytes in the corrupted header; a clobbered NimBLE host mutex tripping xQueueSemaphoreTake). Verified fixed under CONFIG_HEAP_POISONING_COMPREHENSIVE plus periodic heap_caps_check_integrity_all() sweeps. whiteListAdd guards duplicates via onWhiteList(), so at most one element can match - breaking out after the removal preserves semantics. * fix: use m_whiteList.data() - operator[] on an empty vector is UB After erasing the last whitelist entry, &m_whiteList[0] indexes an empty vector before ble_gap_wl_set(..., 0). data() is well-defined for empty vectors; same change applied to the whiteListAdd call site for consistency. --- src/NimBLEDevice.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/NimBLEDevice.cpp b/src/NimBLEDevice.cpp index 313abd2f..e140d620 100644 --- a/src/NimBLEDevice.cpp +++ b/src/NimBLEDevice.cpp @@ -728,7 +728,7 @@ bool NimBLEDevice::onWhiteList(const NimBLEAddress& address) { bool NimBLEDevice::whiteListAdd(const NimBLEAddress& address) { if (!NimBLEDevice::onWhiteList(address)) { m_whiteList.push_back(address); - int rc = ble_gap_wl_set(reinterpret_cast(&m_whiteList[0]), m_whiteList.size()); + int rc = ble_gap_wl_set(reinterpret_cast(m_whiteList.data()), m_whiteList.size()); if (rc != 0) { NIMBLE_LOGE(LOG_TAG, "Failed adding to whitelist rc=%d", rc); m_whiteList.pop_back(); @@ -748,7 +748,7 @@ bool NimBLEDevice::whiteListRemove(const NimBLEAddress& address) { for (auto it = m_whiteList.begin(); it < m_whiteList.end(); ++it) { if (*it == address) { m_whiteList.erase(it); - int rc = ble_gap_wl_set(reinterpret_cast(&m_whiteList[0]), m_whiteList.size()); + int rc = ble_gap_wl_set(reinterpret_cast(m_whiteList.data()), m_whiteList.size()); if (rc != 0) { m_whiteList.push_back(address); NIMBLE_LOGE(LOG_TAG, "Failed removing from whitelist rc=%d", rc); @@ -756,6 +756,10 @@ bool NimBLEDevice::whiteListRemove(const NimBLEAddress& address) { } std::vector(m_whiteList).swap(m_whiteList); + break; // `it` was invalidated by erase() and its buffer freed by the + // swap above; continuing would iterate a dangling iterator. + // Duplicates are impossible (whiteListAdd checks onWhiteList), + // so a single match is total. } } From ef100d63600916a2c3a5186d8ca433d93c396d41 Mon Sep 17 00:00:00 2001 From: h2zero Date: Wed, 29 Jul 2026 16:43:44 -0600 Subject: [PATCH 2/2] [Bugfix] scan response timer causing crash on reinit cycle. Fixes a crash when the stack is reinitialized without clearing all data on deinit then starting scanning. --- src/NimBLEDevice.cpp | 8 ++++++++ src/NimBLEScan.cpp | 41 +++++++++++++++++++++++++++++++++++------ src/NimBLEScan.h | 2 ++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/NimBLEDevice.cpp b/src/NimBLEDevice.cpp index e140d620..e125aeb9 100644 --- a/src/NimBLEDevice.cpp +++ b/src/NimBLEDevice.cpp @@ -1031,6 +1031,11 @@ bool NimBLEDevice::deinit(bool clearAll) { if (m_initialized) { rc = nimble_port_stop(); if (rc == 0) { +# if MYNEWT_VAL(BLE_ROLE_OBSERVER) + if (NimBLEDevice::m_pScan != nullptr) { + NimBLEDevice::m_pScan->onHostDeinit(); + } +# endif nimble_port_deinit(); # ifndef USING_NIMBLE_ARDUINO_HEADERS # if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0) @@ -1042,6 +1047,9 @@ bool NimBLEDevice::deinit(bool clearAll) { # endif m_initialized = false; m_synced = false; + } else { + NIMBLE_LOGE(LOG_TAG, "nimble_port_stop() failed with error: %d", rc); + return false; } } diff --git a/src/NimBLEScan.cpp b/src/NimBLEScan.cpp index 90a381aa..870e233f 100644 --- a/src/NimBLEScan.cpp +++ b/src/NimBLEScan.cpp @@ -84,7 +84,6 @@ NimBLEScan::NimBLEScan() }, m_pTaskData{nullptr}, m_maxResults{0xFF} { - ble_npl_callout_init(&m_srTimer, nimble_port_get_dflt_eventq(), NimBLEScan::srTimerCb, nullptr); ble_npl_time_ms_to_ticks(DEFAULT_SCAN_RESP_TIMEOUT_MS, &m_srTimeoutTicks); } // NimBLEScan::NimBLEScan @@ -92,7 +91,10 @@ NimBLEScan::NimBLEScan() * @brief Scan destructor, release any allocated resources. */ NimBLEScan::~NimBLEScan() { - ble_npl_callout_deinit(&m_srTimer); + if (m_srTimerInitialized) { + ble_npl_callout_deinit(&m_srTimer); + m_srTimerInitialized = false; + } for (const auto& dev : m_scanResults.m_deviceVec) { delete dev; @@ -178,7 +180,9 @@ void NimBLEScan::removeWaitingDevice(NimBLEAdvertisedDevice* pDev) { void NimBLEScan::clearWaitingList() { // Stop the timer and remove any pending timeout events since we're clearing // the list and won't be processing any more timeouts for these devices - ble_npl_callout_stop(&m_srTimer); + if (m_srTimerInitialized) { + ble_npl_callout_stop(&m_srTimer); + } ble_npl_hw_enter_critical(); NimBLEAdvertisedDevice* current = m_pWaitingListHead; while (current != nullptr) { @@ -196,10 +200,19 @@ void NimBLEScan::clearWaitingList() { */ void NimBLEScan::resetWaitingTimer() { if (m_srTimeoutTicks == 0 || m_pWaitingListHead == nullptr) { - ble_npl_callout_stop(&m_srTimer); + if (m_srTimerInitialized) { + ble_npl_callout_stop(&m_srTimer); + } return; } + if (!m_srTimerInitialized) { + m_srTimerInitialized = ble_npl_callout_init(&m_srTimer, nimble_port_get_dflt_eventq(), NimBLEScan::srTimerCb, nullptr) == 0; + if (!m_srTimerInitialized) { + return; + } + } + ble_npl_time_t now = ble_npl_time_get(); ble_npl_time_t elapsed = now - m_pWaitingListHead->m_time; ble_npl_time_t nextTime = elapsed >= m_srTimeoutTicks ? 1 : m_srTimeoutTicks - elapsed; @@ -346,7 +359,9 @@ int NimBLEScan::handleGapEvent(ble_gap_event* event, void* arg) { } case BLE_GAP_EVENT_DISC_COMPLETE: { - ble_npl_callout_stop(&pScan->m_srTimer); + if (pScan->m_srTimerInitialized) { + ble_npl_callout_stop(&pScan->m_srTimer); + } // If we have any scannable devices that haven't received a scan response, // we should trigger the callback with whatever data we have since the scan is complete @@ -392,7 +407,9 @@ int NimBLEScan::handleGapEvent(ble_gap_event* event, void* arg) { */ void NimBLEScan::setScanResponseTimeout(uint32_t timeoutMs) { if (timeoutMs == 0) { - ble_npl_callout_stop(&m_srTimer); + if (m_srTimerInitialized) { + ble_npl_callout_stop(&m_srTimer); + } m_srTimeoutTicks = 0; return; } @@ -673,6 +690,18 @@ void NimBLEScan::onHostSync() { m_pScanCallbacks->onScanEnd(m_scanResults, BLE_HS_ENOTSYNCED); } +/** + * @brief Called before host deinit so callouts don't outlive the default event queue. + */ +void NimBLEScan::onHostDeinit() { + clearWaitingList(); + + if (m_srTimerInitialized) { + ble_npl_callout_deinit(&m_srTimer); + m_srTimerInitialized = false; + } +} + /** * @brief Start scanning and block until scanning has been completed. * @param [in] duration The duration in milliseconds for which to scan. diff --git a/src/NimBLEScan.h b/src/NimBLEScan.h index 0c612851..89c858c8 100644 --- a/src/NimBLEScan.h +++ b/src/NimBLEScan.h @@ -181,6 +181,7 @@ class NimBLEScan { ~NimBLEScan(); static int handleGapEvent(ble_gap_event* event, void* arg); void onHostSync(); + void onHostDeinit(); static void srTimerCb(ble_npl_event* event); // Linked list helpers for devices awaiting scan responses @@ -194,6 +195,7 @@ class NimBLEScan { NimBLEScanResults m_scanResults; NimBLEUtils::TaskData* m_pTaskData; ble_npl_callout m_srTimer{}; + bool m_srTimerInitialized{false}; ble_npl_time_t m_srTimeoutTicks{}; uint8_t m_maxResults; NimBLEAdvertisedDevice* m_pWaitingListHead{}; // head of linked list for devices awaiting scan responses