From 26b2cd2a134146780708b062ac72f5659bca3bdc Mon Sep 17 00:00:00 2001 From: Shreyas Sheth Date: Tue, 24 Mar 2026 15:20:26 +0530 Subject: [PATCH 1/3] fix(wpa_supplicant): Fix concurrency issues between wpa3 and wifi task --- .../esp_supplicant/src/esp_hostap.c | 90 ++++++++--- .../esp_supplicant/src/esp_wpa3.c | 151 ++++++++++++++---- components/wpa_supplicant/src/ap/hostapd.h | 21 +++ components/wpa_supplicant/src/ap/ieee802_11.c | 33 +++- components/wpa_supplicant/src/ap/sta_info.c | 83 ++++++++-- components/wpa_supplicant/src/ap/sta_info.h | 6 +- components/wpa_supplicant/src/ap/wpa_auth.c | 20 +++ 7 files changed, 326 insertions(+), 78 deletions(-) diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c b/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c index 4dfd9cc4a1e..78cc4fffd1d 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c @@ -205,7 +205,6 @@ void *hostap_init(void) } #ifdef CONFIG_SAE - dl_list_init(&hapd->sae_commit_queue); auth_conf->sae_require_mfp = 1; #endif /* CONFIG_SAE */ @@ -225,6 +224,17 @@ void *hostap_init(void) return (void *)hapd; 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(); + } + os_mutex_delete(hapd->sta_list_lock); + hapd->sta_list_lock = NULL; + } +#endif /* CONFIG_SAE */ if (hapd->conf->ssid.wpa_passphrase != NULL) { os_free(hapd->conf->ssid.wpa_passphrase); } @@ -254,19 +264,21 @@ 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; - if (!dl_list_empty(&hapd->sae_commit_queue)) { + 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) { @@ -457,14 +469,31 @@ send_resp: #ifdef CONFIG_WPS_REGISTRAR static void ap_free_sta_timeout(void *ctx, void *data) { - struct hostapd_data *hapd = (struct hostapd_data *) ctx; - u8 *addr = (u8 *) data; - struct sta_info *sta = ap_get_sta(hapd, addr); + struct hostapd_data *hapd = (struct hostapd_data *)ctx; + u8 *addr = (u8 *)data; + struct sta_info *sta; + HOSTAPD_STA_LIST_LOCK(hapd); + sta = ap_get_sta_internal(hapd, addr); if (sta) { +#ifdef CONFIG_SAE + if (sta->lock) { + if (os_semphr_take(sta->lock, 0)) { + HOSTAPD_STA_LIST_UNLOCK(hapd); + ap_free_sta(hapd, sta); + } else { + sta->remove_pending = true; + HOSTAPD_STA_LIST_UNLOCK(hapd); + } + goto done; + } +#endif /* CONFIG_SAE */ + HOSTAPD_STA_LIST_UNLOCK(hapd); ap_free_sta(hapd, sta); + } else { + HOSTAPD_STA_LIST_UNLOCK(hapd); } - +done: os_free(addr); } #endif @@ -472,42 +501,51 @@ static void ap_free_sta_timeout(void *ctx, void *data) bool wpa_ap_remove(u8* bssid) { struct hostapd_data *hapd = hostapd_get_hapd_data(); + struct sta_info *sta; if (!hapd) { return false; } - struct sta_info *sta = ap_get_sta(hapd, bssid); + + HOSTAPD_STA_LIST_LOCK(hapd); + sta = ap_get_sta_internal(hapd, bssid); if (!sta) { + HOSTAPD_STA_LIST_UNLOCK(hapd); return false; } -#ifdef CONFIG_SAE - if (sta->lock) { - if (os_semphr_take(sta->lock, 0)) { - ap_free_sta(hapd, sta); - } else { - sta->remove_pending = true; - } - return true; - } -#endif /* CONFIG_SAE */ - #ifdef CONFIG_WPS_REGISTRAR - wpa_printf(MSG_DEBUG, "wps_status=%d", wps_get_status()); if (wps_get_status() == WPS_STATUS_PENDING) { + HOSTAPD_STA_LIST_UNLOCK(hapd); u8 *addr = os_malloc(ETH_ALEN); if (!addr) { return false; } - os_memcpy(addr, sta->addr, ETH_ALEN); + os_memcpy(addr, bssid, ETH_ALEN); if (eloop_register_timeout(0, 10000, ap_free_sta_timeout, hapd, addr) != 0) { os_free(addr); return false; } - } else -#endif - ap_free_sta(hapd, sta); + return true; + } +#endif /* CONFIG_WPS_REGISTRAR */ + +#ifdef CONFIG_SAE + if (sta->lock) { + if (os_semphr_take(sta->lock, 0)) { + HOSTAPD_STA_LIST_UNLOCK(hapd); + ap_free_sta(hapd, sta); + } else { + sta->remove_pending = true; + HOSTAPD_STA_LIST_UNLOCK(hapd); + } + return true; + } +#endif /* CONFIG_SAE */ + + HOSTAPD_STA_LIST_UNLOCK(hapd); + ap_free_sta(hapd, sta); return true; } diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c index f1ca4b8143d..908724c9d12 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c @@ -197,6 +197,11 @@ static esp_err_t wpa3_build_sae_confirm(void) void esp_wpa3_free_sae_data(void) { + if (g_sae_token) { + wpabuf_free(g_sae_token); + g_sae_token = NULL; + } + if (g_sae_commit) { wpabuf_free(g_sae_commit); g_sae_commit = NULL; @@ -436,13 +441,20 @@ int wpa3_hostap_post_evt(uint32_t evt_id, uint32_t data) wpa_printf(MSG_DEBUG, "g_wpa3_hostap_auth_api_lock not found"); return ESP_FAIL; } - if (evt.id == SIG_WPA3_RX_CONFIRM || evt.id == SIG_TASK_DEL) { + if (evt.id == SIG_WPA3_RX_CONFIRM) { /* prioritising confirm for completing handshake for committed sta */ if (os_queue_send_to_front(g_wpa3_hostap_evt_queue, &evt, 0) != pdPASS) { WPA3_HOSTAP_AUTH_API_UNLOCK(); wpa_printf(MSG_DEBUG, "failed to add msg to queue front"); return ESP_FAIL; } + } else if (evt.id == SIG_TASK_DEL) { + /* wait for portMAX_DELAY as sometimes queue might be full */ + 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"); + return ESP_FAIL; + } } else { if (os_queue_send(g_wpa3_hostap_evt_queue, &evt, 0) != pdPASS) { WPA3_HOSTAP_AUTH_API_UNLOCK(); @@ -463,17 +475,26 @@ static void wpa3_process_rx_commit(wpa3_hostap_auth_event_t *evt) struct hostapd_data *hapd = (struct hostapd_data *)esp_wifi_get_hostap_private_internal(); struct sta_info *sta = NULL; int ret; + + if (!hapd || !hapd->sta_list_lock) { + wpa_printf(MSG_ERROR, "hapd or sta_list_lock not initialized in %s", __func__); + return; + } + HOSTAPD_STA_LIST_LOCK(hapd); frm = dl_list_first(&hapd->sae_commit_queue, struct hostapd_sae_commit_queue, list); if (!frm) { + HOSTAPD_STA_LIST_UNLOCK(hapd); return; } dl_list_del(&frm->list); + wpa_printf(MSG_DEBUG, "SAE: Process next available message from queue"); - sta = ap_get_sta(hapd, frm->bssid); + sta = ap_get_sta_internal(hapd, frm->bssid); if (!sta) { + HOSTAPD_STA_LIST_UNLOCK(hapd); sta = ap_sta_add(hapd, frm->bssid); if (!sta) { wpa_printf(MSG_DEBUG, "ap_sta_add() failed"); @@ -483,17 +504,28 @@ static void wpa3_process_rx_commit(wpa3_hostap_auth_event_t *evt) 0) != 0) { wpa_printf(MSG_INFO, "esp_send_sae_auth_reply: send failed"); } - goto free; + os_free(frm); + return; + } + HOSTAPD_STA_LIST_LOCK(hapd); + sta = ap_get_sta_internal(hapd, frm->bssid); + if (!sta) { + HOSTAPD_STA_LIST_UNLOCK(hapd); + os_free(frm); + return; } } if (sta->lock && os_semphr_take(sta->lock, 0)) { + HOSTAPD_STA_LIST_UNLOCK(hapd); + sta->sae_commit_processing = true; ret = handle_auth_sae(hapd, sta, frm->msg, frm->len, frm->bssid, frm->auth_transaction, frm->status); if (sta->remove_pending) { ap_free_sta(hapd, sta); - goto free; + os_free(frm); + return; } sta->sae_commit_processing = false; os_semphr_give(sta->lock); @@ -505,9 +537,11 @@ static void wpa3_process_rx_commit(wpa3_hostap_auth_event_t *evt) esp_wifi_ap_deauth_internal(frm->bssid, ret); } } + os_free(frm); + return; } -free: + HOSTAPD_STA_LIST_UNLOCK(hapd); os_free(frm); } @@ -517,45 +551,70 @@ static void wpa3_process_rx_confirm(wpa3_hostap_auth_event_t *evt) struct sta_info *sta = NULL; int ret = WLAN_STATUS_SUCCESS; struct sae_hostap_confirm_data *frm = (struct sae_hostap_confirm_data *)evt->data; + if (!frm) { return; } - sta = ap_get_sta(hapd, frm->bssid); - if (!sta) { + + if (!hapd || !hapd->sta_list_lock) { + wpa_printf(MSG_ERROR, "hapd or sta_list_lock not initialized in %s", __func__); os_free(frm); return; } - if (sta->lock && os_semphr_take(sta->lock, 0)) { - ret = handle_auth_sae(hapd, sta, frm->msg, frm->len, frm->bssid, frm->auth_transaction, frm->status); + HOSTAPD_STA_LIST_LOCK(hapd); + sta = ap_get_sta_internal(hapd, frm->bssid); + if (!sta) { + HOSTAPD_STA_LIST_UNLOCK(hapd); + os_free(frm); + return; + } - if (sta->remove_pending) { + if (!sta->lock) { + wpa_printf(MSG_DEBUG, "SAE: sta->lock is NULL for " MACSTR, MAC2STR(frm->bssid)); + HOSTAPD_STA_LIST_UNLOCK(hapd); + os_free(frm); + return; + } + + if (os_semphr_take(sta->lock, 0) != pdTRUE) { + HOSTAPD_STA_LIST_UNLOCK(hapd); + os_free(frm); + return; + } + + HOSTAPD_STA_LIST_UNLOCK(hapd); + + ret = handle_auth_sae(hapd, sta, frm->msg, frm->len, frm->bssid, frm->auth_transaction, frm->status); + + if (sta->remove_pending) { + ap_free_sta(hapd, sta); + goto done; + } + + if (ret == WLAN_STATUS_SUCCESS) { + if (sta->sae_data && esp_send_sae_auth_reply(hapd, sta->addr, frm->bssid, WLAN_AUTH_SAE, 2, + WLAN_STATUS_SUCCESS, wpabuf_head(sta->sae_data), wpabuf_len(sta->sae_data)) != ESP_OK) { ap_free_sta(hapd, sta); goto done; } - if (ret == WLAN_STATUS_SUCCESS) { - if (sta->sae_data && esp_send_sae_auth_reply(hapd, sta->addr, frm->bssid, WLAN_AUTH_SAE, 2, - WLAN_STATUS_SUCCESS, wpabuf_head(sta->sae_data), wpabuf_len(sta->sae_data)) != ESP_OK) { - ap_free_sta(hapd, sta); - goto done; - } - if (esp_wifi_ap_notify_node_sae_auth_done(frm->bssid) != true) { - ap_free_sta(hapd, sta); - goto done; - } + if (esp_wifi_ap_notify_node_sae_auth_done(frm->bssid) != true) { + ap_free_sta(hapd, sta); + goto done; } os_semphr_give(sta->lock); - if (ret != WLAN_STATUS_SUCCESS) { - uint16_t aid = 0; - esp_wifi_ap_get_sta_aid(frm->bssid, &aid); - if (aid == 0) { - esp_wifi_ap_deauth_internal(frm->bssid, ret); - } else { - if (sta && sta->sae_data) { - wpabuf_free(sta->sae_data); - sta->sae_data = NULL; - } + } else { + uint16_t aid = 0; + esp_wifi_ap_get_sta_aid(frm->bssid, &aid); + if (aid == 0) { + os_semphr_give(sta->lock); + esp_wifi_ap_deauth_internal(frm->bssid, ret); + } else { + if (sta->sae_data) { + wpabuf_free(sta->sae_data); + sta->sae_data = NULL; } + os_semphr_give(sta->lock); } } done: @@ -610,22 +669,33 @@ static void esp_wpa3_hostap_task(void *pvParameters) int wpa3_hostap_auth_init(void *data) { + struct hostapd_data *hapd = (struct hostapd_data *)data; if (g_wpa3_hostap_evt_queue) { wpa_printf(MSG_ERROR, "esp_wpa3_hostap_task has already been initialised"); return ESP_OK; } + hapd->sta_list_lock = os_mutex_create(); + if (!hapd->sta_list_lock) { + wpa_printf(MSG_ERROR, "wpa3_hostap_auth_init: failed to create sta_list_lock"); + return ESP_FAIL; + } + 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) { wpa_printf(MSG_ERROR, "wpa3_hostap_auth_init: failed to create WPA3 hostap auth API lock"); + os_mutex_delete(hapd->sta_list_lock); + hapd->sta_list_lock = NULL; return ESP_FAIL; } } - g_wpa3_hostap_evt_queue = os_queue_create(10, sizeof(wpa3_hostap_auth_event_t)); + g_wpa3_hostap_evt_queue = os_queue_create(10, sizeof(wpa3_hostap_auth_event_t)); if (!g_wpa3_hostap_evt_queue) { wpa_printf(MSG_ERROR, "wpa3_hostap_auth_init: failed to create queue"); + os_mutex_delete(hapd->sta_list_lock); + hapd->sta_list_lock = NULL; return ESP_FAIL; } @@ -636,9 +706,13 @@ int wpa3_hostap_auth_init(void *data) wpa_printf(MSG_ERROR, "wpa3_hostap_auth_init: failed to create task"); os_queue_delete(g_wpa3_hostap_evt_queue); g_wpa3_hostap_evt_queue = NULL; + os_mutex_delete(hapd->sta_list_lock); + hapd->sta_list_lock = NULL; return ESP_FAIL; } + dl_list_init(&hapd->sae_commit_queue); + return ESP_OK; } @@ -658,16 +732,25 @@ static int wpa3_hostap_handle_auth(u8 *buf, size_t len, u32 auth_transaction, u1 if (!hapd) { return ESP_FAIL; } - struct sta_info *sta = ap_get_sta(hapd, bssid); + if (auth_transaction == SAE_MSG_COMMIT) { + HOSTAPD_STA_LIST_LOCK(hapd); + struct sta_info *sta = ap_get_sta_internal(hapd, bssid); if (sta && sta->sae_commit_processing) { - /* Ignore commit msg as we are already processing commit msg for this station */ + HOSTAPD_STA_LIST_UNLOCK(hapd); return ESP_OK; } + HOSTAPD_STA_LIST_UNLOCK(hapd); return auth_sae_queue(hapd, buf, len, bssid, status, auth_transaction); } - if (sta && auth_transaction == SAE_MSG_CONFIRM) { + if (auth_transaction == SAE_MSG_CONFIRM) { + HOSTAPD_STA_LIST_LOCK(hapd); + bool sta_exists = (ap_get_sta_internal(hapd, bssid) != NULL); + HOSTAPD_STA_LIST_UNLOCK(hapd); + if (!sta_exists) { + return ESP_OK; + } struct sae_hostap_confirm_data *frm = os_malloc(sizeof(struct sae_hostap_confirm_data) + len); if (!frm) { wpa_printf(MSG_ERROR, "failed to allocate memory for confirm event"); diff --git a/components/wpa_supplicant/src/ap/hostapd.h b/components/wpa_supplicant/src/ap/hostapd.h index 68160a584c2..ec8a02fb6fa 100644 --- a/components/wpa_supplicant/src/ap/hostapd.h +++ b/components/wpa_supplicant/src/ap/hostapd.h @@ -107,6 +107,10 @@ struct hostapd_data { struct sta_info *sta_hash[STA_HASH_SIZE]; int num_sta; /* number of entries in sta_list */ +#ifdef ESP_SUPPLICANT + void *sta_list_lock; +#endif /* ESP_SUPPLICANT */ + struct eapol_authenticator *eapol_auth; struct wpa_authenticator *wpa_auth; @@ -169,4 +173,21 @@ const struct hostapd_eap_user * hostapd_get_eap_user(struct hostapd_data *hapd, const u8 *identity, size_t identity_len, int phase2); +#ifdef ESP_SUPPLICANT +#define HOSTAPD_STA_LIST_LOCK(hapd) \ + do { \ + if ((hapd) && (hapd)->sta_list_lock) \ + os_mutex_lock((hapd)->sta_list_lock); \ + } while (0) + +#define HOSTAPD_STA_LIST_UNLOCK(hapd) \ + do { \ + if ((hapd) && (hapd)->sta_list_lock) \ + os_mutex_unlock((hapd)->sta_list_lock); \ + } while (0) +#else +#define HOSTAPD_STA_LIST_LOCK(hapd) do { } while (0) +#define HOSTAPD_STA_LIST_UNLOCK(hapd) do { } while (0) +#endif /* ESP_SUPPLICANT */ + #endif /* HOSTAPD_H */ diff --git a/components/wpa_supplicant/src/ap/ieee802_11.c b/components/wpa_supplicant/src/ap/ieee802_11.c index 786987eb80f..141e85343f5 100644 --- a/components/wpa_supplicant/src/ap/ieee802_11.c +++ b/components/wpa_supplicant/src/ap/ieee802_11.c @@ -204,6 +204,10 @@ static int use_sae_anti_clogging(struct hostapd_data *hapd) return 1; } +#ifdef ESP_SUPPLICANT + HOSTAPD_STA_LIST_LOCK(hapd); +#endif /* ESP_SUPPLICANT */ + for (sta = hapd->sta_list; sta; sta = sta->next) { if (sta->sae && (sta->sae->state == SAE_COMMITTED || @@ -211,6 +215,9 @@ static int use_sae_anti_clogging(struct hostapd_data *hapd) open++; } if (open >= hapd->conf->sae_anti_clogging_threshold) { +#ifdef ESP_SUPPLICANT + HOSTAPD_STA_LIST_UNLOCK(hapd); +#endif /* ESP_SUPPLICANT */ return 1; } } @@ -220,9 +227,16 @@ static int use_sae_anti_clogging(struct hostapd_data *hapd) * potentially result in too many open sessions. */ if (open + dl_list_len(&hapd->sae_commit_queue) >= hapd->conf->sae_anti_clogging_threshold) { +#ifdef ESP_SUPPLICANT + HOSTAPD_STA_LIST_UNLOCK(hapd); +#endif /* ESP_SUPPLICANT */ return 1; } +#ifdef ESP_SUPPLICANT + HOSTAPD_STA_LIST_UNLOCK(hapd); +#endif /* ESP_SUPPLICANT */ + return 0; } @@ -681,11 +695,23 @@ int auth_sae_queue(struct hostapd_data *hapd, struct hostapd_sae_commit_queue *q, *q2; unsigned int queue_len; +#ifdef ESP_SUPPLICANT + if (!hapd->sta_list_lock) { + wpa_printf(MSG_DEBUG, + "SAE: sta_list_lock not set (no WPA3 hostap path), drop from " MACSTR, + MAC2STR(bssid)); + return -1; + } + HOSTAPD_STA_LIST_LOCK(hapd); +#endif /* ESP_SUPPLICANT */ queue_len = dl_list_len(&hapd->sae_commit_queue); if (queue_len >= hapd->conf->max_num_sta) { wpa_printf(MSG_DEBUG, "SAE: No more room in message queue - drop the new frame from " MACSTR, MAC2STR(bssid)); +#ifdef ESP_SUPPLICANT + HOSTAPD_STA_LIST_UNLOCK(hapd); +#endif /* ESP_SUPPLICANT */ return 0; } @@ -694,6 +720,9 @@ int auth_sae_queue(struct hostapd_data *hapd, queue_len); q = os_zalloc(sizeof(*q) + len); if (!q) { +#ifdef ESP_SUPPLICANT + HOSTAPD_STA_LIST_UNLOCK(hapd); +#endif /* ESP_SUPPLICANT */ return -1; } @@ -731,11 +760,13 @@ queued: if (wpa3_hostap_post_evt(SIG_WPA3_RX_COMMIT, 0) != 0) { wpa_printf(MSG_ERROR, "failed to queue commit build event"); dl_list_del(&q->list); + HOSTAPD_STA_LIST_UNLOCK(hapd); os_free(q); return -1; } - return 0; + HOSTAPD_STA_LIST_UNLOCK(hapd); #endif /* ESP_SUPPLICANT */ + return 0; } diff --git a/components/wpa_supplicant/src/ap/sta_info.c b/components/wpa_supplicant/src/ap/sta_info.c index 28daefb5479..91e52b25f6e 100644 --- a/components/wpa_supplicant/src/ap/sta_info.c +++ b/components/wpa_supplicant/src/ap/sta_info.c @@ -23,17 +23,30 @@ static void ap_sta_delayed_1x_auth_fail_cb(void *eloop_ctx, void *timeout_ctx); void hostapd_wps_eap_completed(struct hostapd_data *hapd); +/* + * CAUTION: cb is invoked while HOSTAPD_STA_LIST_LOCK is held. + * The mutex is non-recursive (os_mutex_create), so cb must NEVER + * call ap_get_sta, ap_free_sta, ap_sta_add, or any other function + * that acquires HOSTAPD_STA_LIST_LOCK — doing so will deadlock. + * Use the lock-free variants (e.g. ap_get_sta_internal) if needed. + */ int ap_for_each_sta(struct hostapd_data *hapd, int (*cb)(struct hostapd_data *hapd, struct sta_info *sta, void *ctx), void *ctx) { struct sta_info *sta; + int ret; + HOSTAPD_STA_LIST_LOCK(hapd); for (sta = hapd->sta_list; sta; sta = sta->next) { - if (cb(hapd, sta, ctx)) - return 1; + if (cb(hapd, sta, ctx)) { + ret = 1; + HOSTAPD_STA_LIST_UNLOCK(hapd); + return ret; + } } + HOSTAPD_STA_LIST_UNLOCK(hapd); return 0; } @@ -43,9 +56,11 @@ struct sta_info * ap_get_sta(struct hostapd_data *hapd, const u8 *sta) { struct sta_info *s; + HOSTAPD_STA_LIST_LOCK(hapd); s = hapd->sta_hash[STA_HASH(sta)]; while (s != NULL && os_memcmp(s->addr, sta, 6) != 0) s = s->hnext; + HOSTAPD_STA_LIST_UNLOCK(hapd); return s; } @@ -69,6 +84,17 @@ static void ap_sta_list_del(struct hostapd_data *hapd, struct sta_info *sta) tmp->next = sta->next; } +/* Caller MUST hold HOSTAPD_STA_LIST_LOCK (see sta_info.h). */ +struct sta_info * ap_get_sta_internal(struct hostapd_data *hapd, const u8 *sta) +{ + struct sta_info *s; + + s = hapd->sta_hash[STA_HASH(sta)]; + while (s != NULL && os_memcmp(s->addr, sta, 6) != 0) + s = s->hnext; + return s; +} + void ap_sta_hash_add(struct hostapd_data *hapd, struct sta_info *sta) { @@ -100,10 +126,12 @@ static void ap_sta_hash_del(struct hostapd_data *hapd, struct sta_info *sta) void ap_free_sta(struct hostapd_data *hapd, struct sta_info *sta) { + HOSTAPD_STA_LIST_LOCK(hapd); ap_sta_hash_del(hapd, sta); ap_sta_list_del(hapd, sta); hapd->num_sta--; + HOSTAPD_STA_LIST_UNLOCK(hapd); #ifdef CONFIG_SAE sae_clear_data(sta->sae); @@ -113,10 +141,12 @@ void ap_free_sta(struct hostapd_data *hapd, struct sta_info *sta) os_mutex_delete(sta->lock); sta->lock = NULL; } +#ifdef ESP_SUPPLICANT if (sta->sae_data) { wpabuf_free(sta->sae_data); sta->sae_data = NULL; } +#endif /* ESP_SUPPLICANT */ #endif /* CONFIG_SAE */ wpa_auth_sta_deinit(sta->wpa_sm); #ifdef CONFIG_WPS_REGISTRAR @@ -132,18 +162,28 @@ void ap_free_sta(struct hostapd_data *hapd, struct sta_info *sta) } +/* Called during teardown after WPA3 task is stopped, so sta->lock is always available. */ void hostapd_free_stas(struct hostapd_data *hapd) { - struct sta_info *sta, *prev; + struct sta_info *sta; - sta = hapd->sta_list; - - while (sta) { - prev = sta; - sta = sta->next; - wpa_printf(MSG_DEBUG, "Removing station " MACSTR, - MAC2STR(prev->addr)); - ap_free_sta(hapd, prev); + while (1) { + HOSTAPD_STA_LIST_LOCK(hapd); + sta = hapd->sta_list; + if (!sta) { + HOSTAPD_STA_LIST_UNLOCK(hapd); + break; + } +#ifdef CONFIG_SAE + if (sta->lock) { + os_semphr_take(sta->lock, OS_BLOCK); + HOSTAPD_STA_LIST_UNLOCK(hapd); + ap_free_sta(hapd, sta); + continue; + } +#endif /* CONFIG_SAE */ + HOSTAPD_STA_LIST_UNLOCK(hapd); + ap_free_sta(hapd, sta); } } @@ -152,35 +192,48 @@ struct sta_info * ap_sta_add(struct hostapd_data *hapd, const u8 *addr) { struct sta_info *sta; - sta = ap_get_sta(hapd, addr); - if (sta) + HOSTAPD_STA_LIST_LOCK(hapd); + sta = ap_get_sta_internal(hapd, addr); + if (sta) { + HOSTAPD_STA_LIST_UNLOCK(hapd); return sta; + } wpa_printf(MSG_DEBUG, " New STA"); if (hapd->num_sta >= hapd->conf->max_num_sta) { - /* FIX: might try to remove some old STAs first? */ wpa_printf(MSG_DEBUG, "no more room for new STAs (%d/%d)", hapd->num_sta, hapd->conf->max_num_sta); + HOSTAPD_STA_LIST_UNLOCK(hapd); return NULL; } sta = os_zalloc(sizeof(struct sta_info)); if (sta == NULL) { wpa_printf(MSG_ERROR, "malloc failed"); + HOSTAPD_STA_LIST_UNLOCK(hapd); return NULL; } /* initialize STA info data */ os_memcpy(sta->addr, addr, ETH_ALEN); - sta->next = hapd->sta_list; #ifdef CONFIG_SAE sta->sae_commit_processing = false; sta->remove_pending = false; sta->lock = os_semphr_create(1, 1); + if (!sta->lock) { + wpa_printf(MSG_ERROR, "Failed to create sta->lock for " MACSTR, + MAC2STR(addr)); + HOSTAPD_STA_LIST_UNLOCK(hapd); + os_free(sta); + return NULL; + } #endif /* CONFIG_SAE */ + sta->next = hapd->sta_list; hapd->sta_list = sta; hapd->num_sta++; ap_sta_hash_add(hapd, sta); + HOSTAPD_STA_LIST_UNLOCK(hapd); + return sta; } diff --git a/components/wpa_supplicant/src/ap/sta_info.h b/components/wpa_supplicant/src/ap/sta_info.h index 3c3769dd1af..b05b3cdd111 100644 --- a/components/wpa_supplicant/src/ap/sta_info.h +++ b/components/wpa_supplicant/src/ap/sta_info.h @@ -62,9 +62,9 @@ struct sta_info { #ifdef CONFIG_SAE void *lock; struct sae_data *sae; - bool sae_commit_processing; /* halt queuing commit while we are + volatile bool sae_commit_processing; /* halt queuing commit while we are * processing commit for that station */ - bool remove_pending; /* Flag to indicate to free station when + volatile bool remove_pending; /* Flag to indicate to free station when * whose mutex is taken by task */ struct wpabuf *sae_data; #endif /* CONFIG_SAE */ @@ -96,6 +96,8 @@ int ap_for_each_sta(struct hostapd_data *hapd, void *ctx), void *ctx); struct sta_info * ap_get_sta(struct hostapd_data *hapd, const u8 *sta); +/* Caller must hold HOSTAPD_STA_LIST_LOCK(hapd) for the duration of the lookup. */ +struct sta_info * ap_get_sta_internal(struct hostapd_data *hapd, const u8 *sta); void ap_sta_hash_add(struct hostapd_data *hapd, struct sta_info *sta); void ap_free_sta(struct hostapd_data *hapd, struct sta_info *sta); void hostapd_free_stas(struct hostapd_data *hapd); diff --git a/components/wpa_supplicant/src/ap/wpa_auth.c b/components/wpa_supplicant/src/ap/wpa_auth.c index a9e182e2b29..2ec4bcb82b9 100644 --- a/components/wpa_supplicant/src/ap/wpa_auth.c +++ b/components/wpa_supplicant/src/ap/wpa_auth.c @@ -210,10 +210,30 @@ wpa_auth_send_eapol(struct wpa_authenticator *wpa_auth, const u8 *addr, return hostapd_send_eapol(wpa_auth->addr, addr, data, data_len); } +/* + * 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. + */ int wpa_auth_for_each_sta(struct wpa_authenticator *wpa_auth, int (*cb)(struct wpa_state_machine *sm, void *ctx), void *cb_ctx) { + struct hostapd_data *hapd = hostapd_get_hapd_data(); + struct sta_info *sta; + + 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; + } + } + HOSTAPD_STA_LIST_UNLOCK(hapd); return 0; } From 907aac9936c376b3e9e530117a4759af976a12d9 Mon Sep 17 00:00:00 2001 From: Shreyas Sheth Date: Thu, 26 Mar 2026 11:42:05 +0530 Subject: [PATCH 2/3] fix(wpa_supplicant): Address comments for concurrency between WiFi and WPA3 task --- .../esp_supplicant/src/esp_hostap.c | 58 ++++++----- .../esp_supplicant/src/esp_wpa3.c | 34 +++++-- components/wpa_supplicant/src/ap/sta_info.c | 18 ++++ components/wpa_supplicant/src/ap/wpa_auth.c | 99 ++++++++++++++++--- 4 files changed, 168 insertions(+), 41 deletions(-) diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c b/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c index 78cc4fffd1d..1d99405be48 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c +++ b/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 (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 */ diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c index 908724c9d12..c095d23c7e9 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/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) diff --git a/components/wpa_supplicant/src/ap/sta_info.c b/components/wpa_supplicant/src/ap/sta_info.c index 91e52b25f6e..d026bf31c03 100644 --- a/components/wpa_supplicant/src/ap/sta_info.c +++ b/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; diff --git a/components/wpa_supplicant/src/ap/wpa_auth.c b/components/wpa_supplicant/src/ap/wpa_auth.c index 2ec4bcb82b9..8a50d5b21ae 100644 --- a/components/wpa_supplicant/src/ap/wpa_auth.c +++ b/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. - * 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; } From 4c73753c7eff063136614c609ea14f56f127a725 Mon Sep 17 00:00:00 2001 From: Shreyas Sheth Date: Thu, 2 Apr 2026 01:07:28 +0530 Subject: [PATCH 3/3] fix(esp_wifi): Fix concurrency for flags between wpa3 and Wi-Fi task --- .../wpa_supplicant/esp_supplicant/src/esp_hostap.c | 4 ++-- .../wpa_supplicant/esp_supplicant/src/esp_wpa3.c | 10 +++++----- components/wpa_supplicant/src/ap/ieee802_11.c | 10 +++++----- components/wpa_supplicant/src/ap/sta_info.c | 4 ++-- components/wpa_supplicant/src/ap/sta_info.h | 8 ++++++-- components/wpa_supplicant/src/ap/wpa_auth.c | 8 +++++++- 6 files changed, 27 insertions(+), 17 deletions(-) diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c b/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c index 1d99405be48..33be6f39ca6 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c @@ -492,7 +492,7 @@ static void ap_free_sta_timeout(void *ctx, void *data) HOSTAPD_STA_LIST_UNLOCK(hapd); ap_free_sta(hapd, sta); } else { - sta->remove_pending = true; + atomic_store(&sta->remove_pending, true); HOSTAPD_STA_LIST_UNLOCK(hapd); } goto done; @@ -547,7 +547,7 @@ bool wpa_ap_remove(u8* bssid) HOSTAPD_STA_LIST_UNLOCK(hapd); ap_free_sta(hapd, sta); } else { - sta->remove_pending = true; + atomic_store(&sta->remove_pending, true); HOSTAPD_STA_LIST_UNLOCK(hapd); } return true; diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c index c095d23c7e9..f6a6fde8430 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c @@ -519,17 +519,17 @@ static void wpa3_process_rx_commit(wpa3_hostap_auth_event_t *evt) } if (sta->lock && os_semphr_take(sta->lock, 0)) { + atomic_store(&sta->sae_commit_processing, true); HOSTAPD_STA_LIST_UNLOCK(hapd); - sta->sae_commit_processing = true; ret = handle_auth_sae(hapd, sta, frm->msg, frm->len, frm->bssid, frm->auth_transaction, frm->status); - if (sta->remove_pending) { + if (atomic_load(&sta->remove_pending)) { ap_free_sta(hapd, sta); os_free(frm); return; } - sta->sae_commit_processing = false; + atomic_store(&sta->sae_commit_processing, false); os_semphr_give(sta->lock); uint16_t aid = 0; if (ret != WLAN_STATUS_SUCCESS && @@ -589,7 +589,7 @@ static void wpa3_process_rx_confirm(wpa3_hostap_auth_event_t *evt) ret = handle_auth_sae(hapd, sta, frm->msg, frm->len, frm->bssid, frm->auth_transaction, frm->status); - if (sta->remove_pending) { + if (atomic_load(&sta->remove_pending)) { ap_free_sta(hapd, sta); goto done; } @@ -758,7 +758,7 @@ static int wpa3_hostap_handle_auth(u8 *buf, size_t len, u32 auth_transaction, u1 if (auth_transaction == SAE_MSG_COMMIT) { HOSTAPD_STA_LIST_LOCK(hapd); struct sta_info *sta = ap_get_sta_internal(hapd, bssid); - if (sta && sta->sae_commit_processing) { + if (sta && atomic_load(&sta->sae_commit_processing)) { HOSTAPD_STA_LIST_UNLOCK(hapd); return ESP_OK; } diff --git a/components/wpa_supplicant/src/ap/ieee802_11.c b/components/wpa_supplicant/src/ap/ieee802_11.c index 141e85343f5..f952fe1cb1d 100644 --- a/components/wpa_supplicant/src/ap/ieee802_11.c +++ b/components/wpa_supplicant/src/ap/ieee802_11.c @@ -151,7 +151,7 @@ static int auth_sae_send_commit(struct hostapd_data *hapd, } #ifdef ESP_SUPPLICANT - if (sta->remove_pending) { + if (atomic_load(&sta->remove_pending)) { reply_res = -1; } else { reply_res = esp_send_sae_auth_reply(hapd, sta->addr, bssid, WLAN_AUTH_SAE, 1, @@ -177,7 +177,7 @@ static int auth_sae_send_confirm(struct hostapd_data *hapd, } #ifdef ESP_SUPPLICANT - if (sta->remove_pending) { + if (atomic_load(&sta->remove_pending)) { reply_res = -1; wpabuf_free(data); } else { @@ -257,7 +257,7 @@ void sae_accept_sta(struct hostapd_data *hapd, struct sta_info *sta) sta->flags |= WLAN_STA_AUTH; #ifdef ESP_SUPPLICANT - sta->sae_commit_processing = false; + atomic_store(&sta->sae_commit_processing, false); #endif /* ESP_SUPPLICANT */ sta->auth_alg = WLAN_AUTH_SAE; @@ -605,7 +605,7 @@ int handle_auth_sae(struct hostapd_data *hapd, struct sta_info *sta, resp = WLAN_STATUS_ANTI_CLOGGING_TOKEN_REQ; #ifdef ESP_SUPPLICANT - sta->sae_commit_processing = false; + atomic_store(&sta->sae_commit_processing, false); #endif /* ESP_SUPPLICANT */ goto reply; @@ -674,7 +674,7 @@ reply: data = wpabuf_alloc_copy(pos, 2); } #ifdef ESP_SUPPLICANT - if (!sta->remove_pending) { + if (!atomic_load(&sta->remove_pending)) { esp_send_sae_auth_reply(hapd, bssid, bssid, WLAN_AUTH_SAE, auth_transaction, resp, data ? wpabuf_head(data) : (u8 *) "", diff --git a/components/wpa_supplicant/src/ap/sta_info.c b/components/wpa_supplicant/src/ap/sta_info.c index d026bf31c03..d376f7d402f 100644 --- a/components/wpa_supplicant/src/ap/sta_info.c +++ b/components/wpa_supplicant/src/ap/sta_info.c @@ -235,8 +235,8 @@ struct sta_info * ap_sta_add(struct hostapd_data *hapd, const u8 *addr) /* initialize STA info data */ os_memcpy(sta->addr, addr, ETH_ALEN); #ifdef CONFIG_SAE - sta->sae_commit_processing = false; - sta->remove_pending = false; + atomic_init(&sta->sae_commit_processing, false); + atomic_init(&sta->remove_pending, false); sta->lock = os_semphr_create(1, 1); if (!sta->lock) { wpa_printf(MSG_ERROR, "Failed to create sta->lock for " MACSTR, diff --git a/components/wpa_supplicant/src/ap/sta_info.h b/components/wpa_supplicant/src/ap/sta_info.h index b05b3cdd111..d4c6d865f43 100644 --- a/components/wpa_supplicant/src/ap/sta_info.h +++ b/components/wpa_supplicant/src/ap/sta_info.h @@ -9,6 +9,10 @@ #ifndef STA_INFO_H #define STA_INFO_H +#ifdef CONFIG_SAE +#include +#endif + /* STA flags */ #define WLAN_STA_AUTH BIT(0) #define WLAN_STA_ASSOC BIT(1) @@ -62,9 +66,9 @@ struct sta_info { #ifdef CONFIG_SAE void *lock; struct sae_data *sae; - volatile bool sae_commit_processing; /* halt queuing commit while we are + atomic_bool sae_commit_processing; /* halt queuing commit while we are * processing commit for that station */ - volatile bool remove_pending; /* Flag to indicate to free station when + atomic_bool remove_pending; /* Flag to indicate to free station when * whose mutex is taken by task */ struct wpabuf *sae_data; #endif /* CONFIG_SAE */ diff --git a/components/wpa_supplicant/src/ap/wpa_auth.c b/components/wpa_supplicant/src/ap/wpa_auth.c index 8a50d5b21ae..744703e1ee2 100644 --- a/components/wpa_supplicant/src/ap/wpa_auth.c +++ b/components/wpa_supplicant/src/ap/wpa_auth.c @@ -301,8 +301,14 @@ int wpa_auth_for_each_sta(struct wpa_authenticator *wpa_auth, if (sta_lk) { HOSTAPD_STA_LIST_LOCK(hapd); sta = ap_get_sta_internal(hapd, sta_mac); - if (sta && sta->lock == sta_lk) + if (sta && sta->lock == sta_lk) { os_semphr_give(sta_lk); + } else if (!sta) { + wpa_printf(MSG_DEBUG, + "WPA: sta->lock not released (STA " MACSTR + " gone); ap_free_sta released semaphore", + MAC2STR(sta_mac)); + } HOSTAPD_STA_LIST_UNLOCK(hapd); sta_lk = NULL; }