fix(esp_wifi): Address review comments on MR

This commit is contained in:
Kapil Gupta
2026-04-12 09:26:35 +05:30
parent 4707f8ee7d
commit fd883d87ac
4 changed files with 117 additions and 61 deletions
@@ -1129,7 +1129,7 @@ esp_err_t esp_supp_dpp_init(void)
return ESP_FAIL;
}
if (is_wps_enabled()) {
if (wps_get_owner() != WPS_OWNER_NONE) {
wpa_printf(MSG_ERROR, "DPP: failed to init since WPS is enabled");
return ESP_FAIL;
}
@@ -4,8 +4,6 @@
* SPDX-License-Identifier: Apache-2.0
*/
#include <stdatomic.h>
#include "utils/common.h"
#include "rsn_supp/wpa.h"
@@ -33,7 +31,6 @@
extern struct wps_sm *gWpsSm;
extern void *s_wps_api_lock;
extern void *s_wps_api_sem;
extern atomic_bool s_wps_enabled;
static int wps_reg_eloop_post_block(uint32_t sig, void *arg);
@@ -147,8 +144,8 @@ int wifi_ap_wps_deinit(void)
static int wifi_ap_wps_enable_internal(const esp_wps_config_t *config)
{
struct wps_sm *sm = gWpsSm;
wifi_mode_t mode = WIFI_MODE_NULL;
enum wps_owner owner;
if (esp_wifi_get_user_init_flag_internal() == 0) {
wpa_printf(MSG_ERROR, "wps enable: wifi not started cannot enable wpsreg");
@@ -170,8 +167,9 @@ static int wifi_ap_wps_enable_internal(const esp_wps_config_t *config)
return ESP_ERR_WIFI_MODE;
}
if (atomic_load(&s_wps_enabled)) {
if (sm && os_memcmp(sm->identity, WSC_ID_ENROLLEE, sm->identity_len) == 0) {
owner = wps_get_owner();
if (owner != WPS_OWNER_NONE) {
if (owner == WPS_OWNER_ENROLLEE) {
wpa_printf(MSG_ERROR, "wps enable: wps enrollee already enabled cannot enable wpsreg");
return ESP_ERR_WIFI_MODE;
} else {
@@ -203,7 +201,7 @@ static int wifi_ap_wps_enable_internal(const esp_wps_config_t *config)
}
wpa_printf(MSG_INFO, "wifi_wps_enable");
atomic_store(&s_wps_enabled, true);
wps_set_owner(WPS_OWNER_REGISTRAR);
return ESP_OK;
_err:
@@ -226,16 +224,15 @@ int esp_wifi_ap_wps_enable(const esp_wps_config_t *config)
int wifi_ap_wps_disable_internal(void)
{
struct wps_sm *sm = gWpsSm;
enum wps_owner owner = wps_get_owner();
if (sm && os_memcmp(sm->identity, WSC_ID_ENROLLEE, sm->identity_len) == 0) {
return ESP_ERR_WIFI_MODE;
}
if (!atomic_load(&s_wps_enabled)) {
if (owner == WPS_OWNER_NONE) {
wpa_printf(MSG_DEBUG, "wps disable: already disabled");
return ESP_OK;
}
if (owner == WPS_OWNER_ENROLLEE) {
return ESP_ERR_WIFI_MODE;
}
wpa_printf(MSG_INFO, "wifi_wps_disable");
if (wps_set_type(WPS_TYPE_DISABLE) != ESP_OK) {
@@ -250,7 +247,7 @@ int wifi_ap_wps_disable_internal(void)
goto _err;
}
atomic_store(&s_wps_enabled, false);
wps_set_owner(WPS_OWNER_NONE);
return ESP_OK;
_err:
@@ -271,6 +268,7 @@ static int wifi_ap_wps_start_internal(const unsigned char *pin)
{
wifi_mode_t mode = WIFI_MODE_NULL;
int prev_wps_status;
enum wps_owner owner;
esp_wifi_get_mode(&mode);
if (mode != WIFI_MODE_AP && mode != WIFI_MODE_APSTA) {
@@ -278,10 +276,15 @@ static int wifi_ap_wps_start_internal(const unsigned char *pin)
return ESP_ERR_WIFI_MODE;
}
if (!atomic_load(&s_wps_enabled)) {
owner = wps_get_owner();
if (owner == WPS_OWNER_NONE) {
wpa_printf(MSG_ERROR, "wps start: wps not enabled");
return ESP_ERR_WIFI_WPS_SM;
}
if (owner != WPS_OWNER_REGISTRAR) {
wpa_printf(MSG_ERROR, "wps start: wps enrollee already enabled");
return ESP_ERR_WIFI_MODE;
}
if (wps_get_type() == WPS_TYPE_DISABLE ||
(wps_get_status() != WPS_STATUS_DISABLE &&
@@ -72,8 +72,8 @@ const char *wps_model_number = CONFIG_IDF_TARGET;
/* API lock to ensure thread safety for public WPS functions */
void *s_wps_api_lock = NULL; /* Used in WPS/WPS-REG public API only, never be freed */
void *s_wps_api_sem = NULL; /* Sync semaphore used between WPS/WPS-REG public API caller task and WPS task, never be freed */
/* Atomic enable flag; API code still uses s_wps_api_lock for compound state checks. */
atomic_bool s_wps_enabled = ATOMIC_VAR_INIT(false);
/* Atomic WPS owner shared between API callers and Wi-Fi task callbacks. */
static atomic_int s_wps_owner = ATOMIC_VAR_INIT(WPS_OWNER_NONE);
static void wifi_wps_scan_done(void *arg, ETS_STATUS status);
static void wifi_wps_scan(void *data, void *user_ctx);
@@ -446,6 +446,52 @@ static int wps_eap_wsc_process_fragment(struct wps_eap_wsc_frag_data *frag,
return 0;
}
static int wps_eap_wsc_prepare_rx_buf(struct wps_sm *sm,
struct wps_eap_wsc_frag_data *frag,
u8 flags, u8 op_code,
u16 message_length,
const u8 *buf, size_t len,
struct wpabuf *tmpbuf,
bool *fragment_pending)
{
*fragment_pending = false;
if (frag->in_buf) {
if (wps_eap_wsc_process_cont(frag, buf, len, op_code) < 0) {
goto fail;
}
if (flags & WSC_FLAGS_MF) {
if (wps_send_frag_ack(sm->current_identifier) != ESP_OK) {
goto fail;
}
*fragment_pending = true;
}
return ESP_OK;
}
if (flags & WSC_FLAGS_MF) {
if (wps_eap_wsc_process_fragment(frag, flags, op_code,
message_length, buf, len) < 0) {
goto fail;
}
if (wps_send_frag_ack(sm->current_identifier) != ESP_OK) {
goto fail;
}
*fragment_pending = true;
return ESP_OK;
}
wpabuf_set(tmpbuf, buf, len);
frag->in_buf = tmpbuf;
return ESP_OK;
fail:
wpabuf_free(frag->in_buf);
frag->in_buf = NULL;
return ESP_FAIL;
}
static void wifi_station_wps_post_m8_timeout(void *data, void *user_ctx)
{
struct wps_sm *sm = wps_sm_get();
@@ -467,6 +513,7 @@ static int wps_process_wps_mX_req(u8 *ubuf, int len, enum wps_process_res *res)
const u8 *pos, *end;
u8 flags;
u16 message_length = 0;
bool fragment_pending;
struct wpabuf tmpbuf;
if (!sm || !res) {
@@ -520,35 +567,17 @@ static int wps_process_wps_mX_req(u8 *ubuf, int len, enum wps_process_res *res)
wpa_printf(MSG_DEBUG, "WPS: Received packet: Op-Code %d Flags 0x%x "
"Message Length %d", expd->opcode, flags, message_length);
if (frag->in_buf &&
wps_eap_wsc_process_cont(frag, pos, end - pos, expd->opcode) < 0) {
wpabuf_free(frag->in_buf);
frag->in_buf = NULL;
if (wps_eap_wsc_prepare_rx_buf(sm, frag, flags, expd->opcode,
message_length, pos, end - pos,
&tmpbuf, &fragment_pending) != ESP_OK) {
return ESP_FAIL;
}
if (flags & WSC_FLAGS_MF) {
if (wps_eap_wsc_process_fragment(frag, flags, expd->opcode,
message_length, pos,
end - pos) < 0) {
wpabuf_free(frag->in_buf);
frag->in_buf = NULL;
return ESP_FAIL;
}
if (wps_send_frag_ack(sm->current_identifier) != ESP_OK) {
wpabuf_free(frag->in_buf);
frag->in_buf = NULL;
return ESP_FAIL;
}
if (fragment_pending) {
*res = WPS_FRAGMENT;
return ESP_OK;
}
if (frag->in_buf == NULL) {
wpabuf_set(&tmpbuf, pos, end - pos);
frag->in_buf = &tmpbuf;
}
eloop_cancel_timeout(wifi_station_wps_msg_timeout, NULL, NULL);
*res = wps_enrollee_process_msg(sm->wps, expd->opcode, frag->in_buf);
@@ -650,7 +679,6 @@ static int wps_send_event_and_disable(int32_t event_id, void* event_data, size_t
esp_event_post(WIFI_EVENT, event_id, event_data, data_len, OS_BLOCK);
atomic_store(&s_wps_enabled, false);
wps_set_type(WPS_TYPE_DISABLE);
wifi_wps_disable_internal(NULL, NULL);
@@ -749,7 +777,6 @@ static int wps_finish(void)
wifi_station_fill_event_info(sm, &s_wps_success_evt);
/* Disable WPS when success */
atomic_store(&s_wps_enabled, false);
wps_set_type(WPS_TYPE_DISABLE);
wifi_wps_disable_internal(NULL, NULL);
@@ -1585,6 +1612,7 @@ static int wps_check_wifi_mode(void)
int esp_wifi_wps_enable(const esp_wps_config_t *config)
{
int ret = ESP_OK;
enum wps_owner owner;
if (esp_wifi_get_user_init_flag_internal() == 0) {
wpa_printf(MSG_ERROR, "WPS: Wi-Fi not started, cannot enable");
@@ -1601,8 +1629,9 @@ int esp_wifi_wps_enable(const esp_wps_config_t *config)
}
API_MUTEX_TAKE();
if (atomic_load(&s_wps_enabled)) {
if (gWpaSm.wpa_sm_wps_disable == NULL) {
owner = wps_get_owner();
if (owner != WPS_OWNER_NONE) {
if (owner == WPS_OWNER_REGISTRAR) {
wpa_printf(MSG_ERROR, "WPS: Cannot enable Enrollee, Registrar is already enabled");
ret = ESP_ERR_WIFI_MODE;
} else {
@@ -1613,16 +1642,18 @@ int esp_wifi_wps_enable(const esp_wps_config_t *config)
}
ret = eloop_register_timeout_blocking(wifi_wps_enable_internal, NULL, (void *)config);
if (ret == ESP_OK) {
atomic_store(&s_wps_enabled, true);
}
API_MUTEX_GIVE();
return ret;
}
bool is_wps_enabled(void)
enum wps_owner wps_get_owner(void)
{
return atomic_load(&s_wps_enabled);
return (enum wps_owner) atomic_load(&s_wps_owner);
}
void wps_set_owner(enum wps_owner owner)
{
atomic_store(&s_wps_owner, owner);
}
static int wifi_wps_disable(void)
@@ -1651,8 +1682,13 @@ static int wifi_wps_enable_internal(void *ctx, void *data)
wpa_printf(MSG_INFO, "WPS: Enabling");
wps_set_type(config->wps_type);
wps_set_status(WPS_STATUS_DISABLE);
if (wps_set_type(config->wps_type) != ESP_OK) {
return ESP_FAIL;
}
if (wps_set_status(WPS_STATUS_DISABLE) != ESP_OK) {
wps_set_type(WPS_TYPE_DISABLE);
return ESP_FAIL;
}
ret = wifi_station_wps_init(config);
@@ -1662,6 +1698,7 @@ static int wifi_wps_enable_internal(void *ctx, void *data)
return ESP_FAIL;
}
wpa_sm->wpa_sm_wps_disable = wifi_wps_disable;
wps_set_owner(WPS_OWNER_ENROLLEE);
return ESP_OK;
}
@@ -1669,6 +1706,7 @@ static int wifi_wps_disable_internal(void *ctx, void *data)
{
/* Only disconnect in case of WPS pending */
gWpaSm.wpa_sm_wps_disable = NULL;
wps_set_owner(WPS_OWNER_NONE);
esp_wifi_set_wps_start_flag_internal(false);
if (wps_get_status() == WPS_STATUS_PENDING) {
@@ -1696,31 +1734,32 @@ int esp_wifi_wps_disable(void)
{
int ret = 0;
int prev_wps_type;
enum wps_owner prev_owner;
API_MUTEX_TAKE();
if (atomic_load(&s_wps_enabled) && gWpaSm.wpa_sm_wps_disable == NULL) {
API_MUTEX_GIVE();
return ESP_ERR_WIFI_MODE;
}
if (!atomic_load(&s_wps_enabled)) {
prev_owner = wps_get_owner();
if (prev_owner == WPS_OWNER_NONE) {
wpa_printf(MSG_DEBUG, "WPS: Already disabled");
API_MUTEX_GIVE();
return ESP_OK;
}
if (prev_owner == WPS_OWNER_REGISTRAR) {
API_MUTEX_GIVE();
return ESP_ERR_WIFI_MODE;
}
wpa_printf(MSG_INFO, "WPS: Disabling");
prev_wps_type = wps_get_type();
wps_set_type(WPS_TYPE_DISABLE); /* Notify WiFi task */
atomic_store(&s_wps_enabled, false);
wps_set_owner(WPS_OWNER_NONE);
ret = eloop_register_timeout_blocking(wifi_wps_disable_internal, NULL, NULL);
if (ESP_OK != ret) {
wpa_printf(MSG_ERROR, "WPS: Failed to disable (ret=%d)", ret);
wps_set_type(prev_wps_type);
atomic_store(&s_wps_enabled, true);
wps_set_owner(prev_owner);
}
API_MUTEX_GIVE();
return ret;
@@ -1729,6 +1768,7 @@ int esp_wifi_wps_disable(void)
int esp_wifi_wps_start(void)
{
int ret;
enum wps_owner owner;
if (ESP_OK != wps_check_wifi_mode()) {
return ESP_ERR_WIFI_MODE;
@@ -1736,11 +1776,17 @@ int esp_wifi_wps_start(void)
API_MUTEX_TAKE();
if (!atomic_load(&s_wps_enabled)) {
owner = wps_get_owner();
if (owner == WPS_OWNER_NONE) {
wpa_printf(MSG_ERROR, "WPS: Cannot start, not enabled");
API_MUTEX_GIVE();
return ESP_ERR_WIFI_WPS_SM;
}
if (owner != WPS_OWNER_ENROLLEE) {
wpa_printf(MSG_ERROR, "WPS: Cannot start Enrollee, Registrar is enabled");
API_MUTEX_GIVE();
return ESP_ERR_WIFI_MODE;
}
if (esp_wifi_get_user_init_flag_internal() == 0) {
API_MUTEX_GIVE();
@@ -18,6 +18,12 @@ enum wps_reg_sig_type {
SIG_WPS_REG_MAX, //4
};
enum wps_owner {
WPS_OWNER_NONE = 0,
WPS_OWNER_ENROLLEE,
WPS_OWNER_REGISTRAR,
};
typedef struct {
void *arg;
int ret; /* return value */
@@ -112,7 +118,8 @@ static inline int wps_set_status(uint32_t status)
return esp_wifi_set_wps_status_internal(status);
}
bool is_wps_enabled(void);
enum wps_owner wps_get_owner(void);
void wps_set_owner(enum wps_owner owner);
int wps_init_cfg_pin(struct wps_config *cfg);
void wifi_station_wps_eapol_start_handle(void *data, void *user_ctx);
int wifi_ap_wps_disable_internal(void);