Aksal/nuttx
3
1
Fork 1
mirror of https://github.com/apache/nuttx.git synced 2026-09-29 10:34:23 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

xtensa/espressif: fix lock-order deadlock in esp_wifi_event_handler()

Browse source 
esp_wifi_event_handler() held esp_wifi_lock() across the whole event
switch, including the esp_wlan_*_hook() calls
(WIFI_EVENT_STA_CONNECTED/_DISCONNECTED, WIFI_EVENT_AP_START/_STOP).
Those hooks reach netdev_lower_carrier_on()/_off(), which take the
per-device netdev_lock().

Every other path into esp_wifi_lock() acquires the two locks in the
opposite order -- the netdev ifdown path holds netdev_lock() around
its own call into esp_wifi_api_stop(), which calls esp_wifi_lock().
An application that disconnects Wi-Fi (wpa_driver_wext_disconnect()
immediately followed by wapi_set_ifdown()) races the resulting
WIFI_EVENT_STA_DISCONNECTED callback against its own ifdown call, and
the two lock orders wedge each other permanently.

Confirmed on real ESP32-S3 hardware (XIAO ESP32-S3,
CONFIG_ESPRESSIF_WIFI + CONFIG_PM + CONFIG_SCHED_TICKLESS): the
disconnecting task and the low-priority work-queue thread each waited
on a mutex held by the other (checked live via JTAG/GDB, not inferred
from code reading alone). Reproduced 4/4 times before this fix, 0/2
after.

Fix: esp_wifi_lock() is now taken only around the specific calls that
reach into the Wi-Fi driver API (esp_wifi_scan_event_parse(),
esp_wifi_set_ps()), never spanning a esp_wlan_*_hook() call --
netdev_lock() first (or absent), esp_wifi_lock() last, on every path.

Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
Assisted-by: Claude:claude-sonnet-5
This commit is contained in:
Felipe Moura 2026-09-17 14:34:52 -03:00 • committed by Xiang Xiao
parent c95c546c09
commit d656cfa930
1 changed files with 12 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

14
arch/xtensa/src/common/espressif/esp_wifi_event_handler.c
View file

@ -150,13 +150,20 @@ static void esp_wifi_event_handler(void *arg, esp_event_base_t event_base,
UNUSED(event_base);
net_lock();
esp_wifi_lock(true);
/* esp_wifi_lock() no longer spans the esp_wlan_*_hook() calls below:
* they take netdev_lock(), which ifdown already holds *around* its own
* esp_wifi_lock() call, so locking both here in the opposite order
* deadlocked ifdown against this handler.
*/
switch (event_id)
{
#ifdef ESP_WLAN_DEVS
case WIFI_EVENT_SCAN_DONE:
esp_wifi_lock(true);
esp_wifi_scan_event_parse();
esp_wifi_lock(false);
break;
#endif
case WIFI_EVENT_HOME_CHANNEL_CHANGE:
@ -168,7 +175,9 @@ static void esp_wifi_event_handler(void *arg, esp_event_base_t event_base,
{
wlinfo("Wi-Fi sta start\n");
esp_wifi_lock(true);
ret = esp_wifi_set_ps(ps_type);
esp_wifi_lock(false);
if (ret != 0)
{
wlerr("Failed to set power save type\n");
@ -214,7 +223,9 @@ static void esp_wifi_event_handler(void *arg, esp_event_base_t event_base,
{
wlinfo("INFO: Wi-Fi softap start\n");
esp_wlan_softap_connect_success_hook();
esp_wifi_lock(true);
ret = esp_wifi_set_ps(ps_type);
esp_wifi_lock(false);
if (ret != 0)
{
wlerr("Failed to set power save type\n");
@ -260,7 +271,6 @@ static void esp_wifi_event_handler(void *arg, esp_event_base_t event_base,
}
}
esp_wifi_lock(false);
net_unlock();
}