fix(wpa_supplicant): Address comments for concurrency between WiFi and WPA3 task

components/wpa_supplicant/esp_supplicant/src/esp_hostap.c

@@ -61,6 +61,9 @@ void *hostap_init(void)
     wifi_pmf_config_t pmf_cfg = {0};
     uint8_t authmode;
     uint8_t sae_ext = 0;
+#ifdef CONFIG_SAE
+    struct hostapd_sae_commit_queue *q, *tmp;
+#endif
 
     sae_ext = esp_wifi_ap_get_sae_ext_config_internal();
 
@@ -226,11 +229,29 @@ void *hostap_init(void)
 fail:
 #ifdef CONFIG_SAE
     if (hapd->sta_list_lock) {
-        wpa3_hostap_auth_deinit();
+        if (wpa3_hostap_auth_deinit()) {
-        if (g_wpa3_hostap_auth_api_lock &&
+            if (g_wpa3_hostap_auth_api_lock) {
-                WPA3_HOSTAP_AUTH_API_LOCK() == pdTRUE) {
+                /* Block until WPA3 task gives the API lock after SIG_TASK_DEL teardown */
+                WPA3_HOSTAP_AUTH_API_LOCK();
                 WPA3_HOSTAP_AUTH_API_UNLOCK();
             }
+        } else {
+            wpa_printf(MSG_ERROR,
+                       "hostap_init fail: failed to post SIG_TASK_DEL, skipping WPA3 API lock wait");
+        }
+        HOSTAPD_STA_LIST_LOCK(hapd);
+        /*
+         * hostap_init() failed before global_hapd was assigned, so the WPA3
+         * hostap task has no registered hapd to drain this queue on exit;
+         * free queued commits here before releasing hapd.
+         */
+        dl_list_for_each_safe(q, tmp, &hapd->sae_commit_queue,
+                              struct hostapd_sae_commit_queue, list) {
+            dl_list_del(&q->list);
+            os_free(q);
+        }
+        HOSTAPD_STA_LIST_UNLOCK(hapd);
+
         os_mutex_delete(hapd->sta_list_lock);
         hapd->sta_list_lock = NULL;
     }
@@ -264,21 +285,6 @@ void hostapd_cleanup(struct hostapd_data *hapd)
         hapd->conf = NULL;
     }
 
-    if (hapd->sta_list_lock) {
-#ifdef CONFIG_SAE
-    struct hostapd_sae_commit_queue *q, *tmp;
-
-    HOSTAPD_STA_LIST_LOCK(hapd);
-        dl_list_for_each_safe(q, tmp, &hapd->sae_commit_queue,
-                              struct hostapd_sae_commit_queue, list) {
-            dl_list_del(&q->list);
-            os_free(q);
-        }
-        HOSTAPD_STA_LIST_UNLOCK(hapd);
-#endif /* CONFIG_SAE */
-        os_mutex_delete(hapd->sta_list_lock);
-        hapd->sta_list_lock = NULL;
-    }
 #ifdef CONFIG_WPS_REGISTRAR
     if (esp_wifi_get_wps_type_internal() != WPS_TYPE_DISABLE ||
             esp_wifi_get_wps_status_internal() != WPS_STATUS_DISABLE) {
@@ -304,12 +310,16 @@ bool hostap_deinit(void *data)
     wifi_ap_wps_disable_internal();
 #endif
 #ifdef CONFIG_SAE
-    wpa3_hostap_auth_deinit();
+    if (wpa3_hostap_auth_deinit()) {
-    /* Wait till lock is released by wpa3 task */
+        /* Block until WPA3 task gives the API lock after SIG_TASK_DEL teardown */
-    if (g_wpa3_hostap_auth_api_lock &&
+        if (g_wpa3_hostap_auth_api_lock) {
-            WPA3_HOSTAP_AUTH_API_LOCK() == pdTRUE) {
+            WPA3_HOSTAP_AUTH_API_LOCK();
             WPA3_HOSTAP_AUTH_API_UNLOCK();
         }
+    } else {
+        wpa_printf(MSG_ERROR,
+                   "hostap_deinit: failed to post SIG_TASK_DEL, skipping WPA3 API lock wait");
+    }
 #endif /* CONFIG_SAE */
 
     hostapd_cleanup(hapd);

components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c

@@ -421,6 +421,7 @@ void esp_wifi_unregister_wpa3_cb(void)
 
 static TaskHandle_t g_wpa3_hostap_task_hdl = NULL;
 static QueueHandle_t g_wpa3_hostap_evt_queue = NULL;
+/* Global API lock - created once, never deleted */
 SemaphoreHandle_t g_wpa3_hostap_auth_api_lock = NULL;
 
 int wpa3_hostap_post_evt(uint32_t evt_id, uint32_t data)
@@ -449,7 +450,8 @@ int wpa3_hostap_post_evt(uint32_t evt_id, uint32_t data)
             return ESP_FAIL;
         }
     } else if (evt.id == SIG_TASK_DEL) {
-        /* wait for portMAX_DELAY as sometimes queue might be full */
+        /* Wi-Fi blocks until SIG_TASK_DEL is queued; only Wi-Fi calls wpa3_hostap_post_evt. */
+        /* Hence there will be no deadlock for g_wpa3_hostap_auth_api_lock. */
         if (os_queue_send_to_front(g_wpa3_hostap_evt_queue, &evt, portMAX_DELAY) != pdPASS) {
             WPA3_HOSTAP_AUTH_API_UNLOCK();
             wpa_printf(MSG_DEBUG, "failed to add msg to queue front");
@@ -660,10 +662,30 @@ static void esp_wpa3_hostap_task(void *pvParameters)
     os_queue_delete(g_wpa3_hostap_evt_queue);
     g_wpa3_hostap_evt_queue = NULL;
 
+    struct hostapd_data *hapd = hostapd_get_hapd_data();
+    if (hapd && hapd->sta_list_lock) {
+        struct hostapd_sae_commit_queue *q, *tmp;
+        HOSTAPD_STA_LIST_LOCK(hapd);
+        dl_list_for_each_safe(q, tmp, &hapd->sae_commit_queue,
+                              struct hostapd_sae_commit_queue, list) {
+            dl_list_del(&q->list);
+            os_free(q);
+        }
+        HOSTAPD_STA_LIST_UNLOCK(hapd);
+        /*
+         * Safe to delete sta_list_lock after unlock: only the WPA3 hostap task and
+         * the Wi-Fi task take this lock; the Wi-Fi task is blocked while posting
+         * SIG_TASK_DEL, so it cannot acquire sta_list_lock again until teardown
+         * sequencing completes.
+         */
+        os_mutex_delete(hapd->sta_list_lock);
+        hapd->sta_list_lock = NULL;
+    }
+
     if (g_wpa3_hostap_auth_api_lock) {
         WPA3_HOSTAP_AUTH_API_UNLOCK();
     }
-    /* At this point, task is deleted*/
+    /* At this point, task is deleted */
     os_task_delete(NULL);
 }
 
@@ -681,6 +703,7 @@ int wpa3_hostap_auth_init(void *data)
         return ESP_FAIL;
     }
 
+    /* g_wpa3_hostap_auth_api_lock is global - created once, never deleted */
     if (g_wpa3_hostap_auth_api_lock == NULL) {
         g_wpa3_hostap_auth_api_lock = os_semphr_create(1, 1);
         if (!g_wpa3_hostap_auth_api_lock) {
@@ -699,6 +722,8 @@ int wpa3_hostap_auth_init(void *data)
         return ESP_FAIL;
     }
 
+    dl_list_init(&hapd->sae_commit_queue);
+
     if (os_task_create(esp_wpa3_hostap_task, "esp_wpa3_hostap_task",
                        WPA3_HOSTAP_HANDLE_AUTH_TASK_STACK_SIZE, NULL,
                        WPA3_HOSTAP_HANDLE_AUTH_TASK_PRIORITY,
@@ -711,8 +736,6 @@ int wpa3_hostap_auth_init(void *data)
         return ESP_FAIL;
     }
 
-    dl_list_init(&hapd->sae_commit_queue);
-
     return ESP_OK;
 }
 
@@ -721,9 +744,8 @@ bool wpa3_hostap_auth_deinit(void)
     if (wpa3_hostap_post_evt(SIG_TASK_DEL, 0) != 0) {
         wpa_printf(MSG_DEBUG, "failed to send task delete event");
         return false;
-    } else {
-        return true;
     }
+    return true;
 }
 
 static int wpa3_hostap_handle_auth(u8 *buf, size_t len, u32 auth_transaction, u16 status, u8 *bssid)

components/wpa_supplicant/src/ap/sta_info.c

@@ -52,6 +52,24 @@ int ap_for_each_sta(struct hostapd_data *hapd,
 }
 
 
+/**
+ * ap_get_sta - Look up a station by MAC address
+ * @hapd: hostapd data structure
+ * @sta: MAC address to look up
+ * Returns: Pointer to sta_info structure, or NULL if not found
+ *
+ * This function acquires and releases HOSTAPD_STA_LIST_LOCK internally.
+ * The returned pointer is NOT protected after the lock is released.
+ * Use-after-free can occur if the station is freed by another task
+ * (via ap_free_sta) between the unlock here and the caller's use.
+ *
+ * For safe access to sta fields, callers should either:
+ * - Use ap_get_sta_internal() while holding HOSTAPD_STA_LIST_LOCK, OR
+ * - Take sta->lock immediately after (for SAE stations), OR
+ * - Copy needed data while lock is still held
+ *
+ * Note: This is particularly important for wpa3_task callers.
+ */
 struct sta_info * ap_get_sta(struct hostapd_data *hapd, const u8 *sta)
 {
 	struct sta_info *s;

components/wpa_supplicant/src/ap/wpa_auth.c

@@ -136,17 +136,26 @@ static inline const u8 * wpa_auth_get_psk(struct wpa_authenticator *wpa_auth,
     }
 
 #ifdef CONFIG_SAE
-    struct sta_info *sta = ap_get_sta(hapd, addr);
+    /* wpa_auth_get_psk runs on the Wi-Fi task only, so sae_pmk_copy is not shared with any other task. */
+    static u8 sae_pmk_copy[PMK_LEN];
+    HOSTAPD_STA_LIST_LOCK(hapd);
+    struct sta_info *sta = ap_get_sta_internal(hapd, addr);
     if (sta && sta->auth_alg == WLAN_AUTH_SAE) {
-        if (!sta->sae || prev_psk)
+        if (!sta->sae || prev_psk) {
+            HOSTAPD_STA_LIST_UNLOCK(hapd);
             return NULL;
-        return sta->sae->pmk;
+        }
+        os_memcpy(sae_pmk_copy, sta->sae->pmk, PMK_LEN);
+        HOSTAPD_STA_LIST_UNLOCK(hapd);
+        return sae_pmk_copy;
     }
     if (sta && wpa_auth_uses_sae(sta->wpa_sm)) {
         wpa_printf(MSG_DEBUG,
                "No PSK for STA trying to use SAE with PMKSA caching");
+        HOSTAPD_STA_LIST_UNLOCK(hapd);
         return NULL;
     }
+    HOSTAPD_STA_LIST_UNLOCK(hapd);
 #endif /*CONFIG_SAE*/
 
     return (u8*)hostapd_get_psk(hapd->conf, addr, prev_psk);
@@ -211,10 +220,10 @@ wpa_auth_send_eapol(struct wpa_authenticator *wpa_auth, const u8 *addr,
 }
 
 /*
- * CAUTION: cb is invoked while HOSTAPD_STA_LIST_LOCK is held.
+ * Modified to handle Wi-Fi vs WPA3 task concurrency only when CONFIG_SAE is enabled.
- * The mutex is non-recursive, so cb must NEVER call any function
+ * Snapshot SM indices under HOSTAPD_STA_LIST_LOCK, then per entry try
- * that acquires HOSTAPD_STA_LIST_LOCK (e.g. ap_get_sta, ap_free_sta)
+ * sta->lock (timeout 0) when present; if it is not acquired, skip
- * — doing so will deadlock.
+ * cb for that station; otherwise run cb.
  */
 int wpa_auth_for_each_sta(struct wpa_authenticator *wpa_auth,
               int (*cb)(struct wpa_state_machine *sm, void *ctx),
@@ -222,18 +231,86 @@ int wpa_auth_for_each_sta(struct wpa_authenticator *wpa_auth,
 {
     struct hostapd_data *hapd = hostapd_get_hapd_data();
     struct sta_info *sta;
+    u8 idx_snap[WPA_SM_MAX_INDEX];
+    unsigned int n = 0;
+    unsigned int i;
+#ifdef CONFIG_SAE
+    void *sta_lk = NULL;
+    u8 sta_mac[ETH_ALEN];
+#endif /* CONFIG_SAE */
+    int cb_ret;
+
 
     if (hapd == NULL)
         return 1;
 
     HOSTAPD_STA_LIST_LOCK(hapd);
     for (sta = hapd->sta_list; sta; sta = sta->next) {
-         if (sta->wpa_sm && cb(sta->wpa_sm, cb_ctx)) {
+        struct wpa_state_machine *sm = sta->wpa_sm;
+
+        if (!sm || n >= WPA_SM_MAX_INDEX)
+            continue;
+        if (sm->index >= WPA_SM_MAX_INDEX)
+            continue;
+        if (!(BIT(sm->index) & s_sm_valid_bitmap))
+            continue;
+        if (s_sm_table[sm->index] != sm)
+            continue;
+        idx_snap[n++] = (u8) sm->index;
+    }
     HOSTAPD_STA_LIST_UNLOCK(hapd);
+
+    for (i = 0; i < n; i++) {
+        struct wpa_state_machine *sm;
+
+#ifdef CONFIG_SAE
+        sta_lk = NULL;
+#endif /* CONFIG_SAE */
+
+        HOSTAPD_STA_LIST_LOCK(hapd);
+        sm = wpa_auth_get_sm(idx_snap[i]);
+        if (!sm) {
+            HOSTAPD_STA_LIST_UNLOCK(hapd);
+            continue;
+        }
+#ifdef CONFIG_SAE
+        sta = ap_get_sta_internal(hapd, sm->addr);
+        if (sta && sta->wpa_sm == sm && sta->lock) {
+            /* Take sta->lock with timeout 0.
+             * Skip cb for this STA if sta->lock is not taken (WPA3 task may be holding the lock). */
+            if (!os_semphr_take(sta->lock, 0)) {
+                HOSTAPD_STA_LIST_UNLOCK(hapd);
+                continue;
+            }
+            sta_lk = sta->lock;
+            os_memcpy(sta_mac, sta->addr, ETH_ALEN);
+        }
+#endif /* CONFIG_SAE */
+        HOSTAPD_STA_LIST_UNLOCK(hapd);
+
+        cb_ret = cb(sm, cb_ctx);
+
+#ifdef CONFIG_SAE
+        /*
+         * Give only when re-lookup finds sta->lock == sta_lk; otherwise skip
+         * os_semphr_give(sta_lk) (sta_lk is not dereferenced on that path).
+         * ap_free_sta() give+deletes sta->lock and clears the field before freeing sta.
+         * ESP softAP: wpa_ap_remove and this walk usually run on the same Wi-Fi task as
+         * cb(), so STA removal does not run concurrently with cb() in the typical model.
+         */
+        if (sta_lk) {
+            HOSTAPD_STA_LIST_LOCK(hapd);
+            sta = ap_get_sta_internal(hapd, sta_mac);
+            if (sta && sta->lock == sta_lk)
+                os_semphr_give(sta_lk);
+            HOSTAPD_STA_LIST_UNLOCK(hapd);
+            sta_lk = NULL;
+        }
+#endif /* CONFIG_SAE */
+
+        if (cb_ret)
             return 1;
     }
-    }
-    HOSTAPD_STA_LIST_UNLOCK(hapd);
     return 0;
 }