fix(esp_wifi): Address review comments on MR

components/wpa_supplicant/esp_supplicant/src/esp_dpp.c

@@ -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;
     }

components/wpa_supplicant/esp_supplicant/src/esp_hostpad_wps.c

@@ -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)) {
+    owner = wps_get_owner();
-        if (sm && os_memcmp(sm->identity, WSC_ID_ENROLLEE, sm->identity_len) == 0) {
+    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) {
+    if (owner == WPS_OWNER_NONE) {
-        return ESP_ERR_WIFI_MODE;
-    }
-
-    if (!atomic_load(&s_wps_enabled)) {
         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 &&

components/wpa_supplicant/esp_supplicant/src/esp_wps.c

@@ -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 WPS owner shared between API callers and Wi-Fi task callbacks. */
-atomic_bool s_wps_enabled = ATOMIC_VAR_INIT(false);
+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 &&
+    if (wps_eap_wsc_prepare_rx_buf(sm, frag, flags, expd->opcode,
-            wps_eap_wsc_process_cont(frag, pos, end - pos, expd->opcode) < 0) {
+                                   message_length, pos, end - pos,
-        wpabuf_free(frag->in_buf);
+                                   &tmpbuf, &fragment_pending) != ESP_OK) {
-        frag->in_buf = NULL;
         return ESP_FAIL;
     }
 
-    if (flags & WSC_FLAGS_MF) {
+    if (fragment_pending) {
-        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;
-        }
         *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);
 
@@ -1594,6 +1621,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");
@@ -1610,8 +1638,9 @@ int esp_wifi_wps_enable(const esp_wps_config_t *config)
     }
 
     API_MUTEX_TAKE();
-    if (atomic_load(&s_wps_enabled)) {
+    owner = wps_get_owner();
-        if (gWpaSm.wpa_sm_wps_disable == NULL) {
+    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 {
@@ -1622,16 +1651,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)
@@ -1660,8 +1691,13 @@ static int wifi_wps_enable_internal(void *ctx, void *data)
 
     wpa_printf(MSG_INFO, "WPS: Enabling");
 
-    wps_set_type(config->wps_type);
+    if (wps_set_type(config->wps_type) != ESP_OK) {
-    wps_set_status(WPS_STATUS_DISABLE);
+        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);
 
@@ -1671,6 +1707,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;
 }
 
@@ -1678,6 +1715,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) {
@@ -1705,31 +1743,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) {
+    prev_owner = wps_get_owner();
-        API_MUTEX_GIVE();
+    if (prev_owner == WPS_OWNER_NONE) {
-        return ESP_ERR_WIFI_MODE;
-    }
-
-    if (!atomic_load(&s_wps_enabled)) {
         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;
@@ -1738,6 +1777,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;
@@ -1745,11 +1785,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();

components/wpa_supplicant/esp_supplicant/src/esp_wps_i.h

@@ -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);