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

This commit is contained in:
Shreyas Sheth
2026-05-11 14:30:59 +05:30
parent 26b2cd2a13
commit 907aac9936
4 changed files with 168 additions and 41 deletions
@@ -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 (g_wpa3_hostap_auth_api_lock &&
WPA3_HOSTAP_AUTH_API_LOCK() == pdTRUE) {
WPA3_HOSTAP_AUTH_API_UNLOCK();
if (wpa3_hostap_auth_deinit()) {
if (g_wpa3_hostap_auth_api_lock) {
/* 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,11 +310,15 @@ bool hostap_deinit(void *data)
wifi_ap_wps_disable_internal();
#endif
#ifdef CONFIG_SAE
wpa3_hostap_auth_deinit();
/* Wait till lock is released by wpa3 task */
if (g_wpa3_hostap_auth_api_lock &&
WPA3_HOSTAP_AUTH_API_LOCK() == pdTRUE) {
WPA3_HOSTAP_AUTH_API_UNLOCK();
if (wpa3_hostap_auth_deinit()) {
/* Block until WPA3 task gives the API lock after SIG_TASK_DEL teardown */
if (g_wpa3_hostap_auth_api_lock) {
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 */
@@ -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)
@@ -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;
+88 -11
View File
@@ -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.
* The mutex is non-recursive, so cb must NEVER call any function
* that acquires HOSTAPD_STA_LIST_LOCK (e.g. ap_get_sta, ap_free_sta)
* — doing so will deadlock.
* Modified to handle Wi-Fi vs WPA3 task concurrency only when CONFIG_SAE is enabled.
* Snapshot SM indices under HOSTAPD_STA_LIST_LOCK, then per entry try
* sta->lock (timeout 0) when present; if it is not acquired, skip
* 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)) {
HOSTAPD_STA_LIST_UNLOCK(hapd);
return 1;
}
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;
}
return 0;
}