fix(esp_wifi): Address review comments on MR

components/wpa_supplicant/esp_supplicant/src/esp_dpp.c

@@ -700,7 +700,7 @@ esp_err_t esp_supp_dpp_init(esp_supp_dpp_event_cb_t cb)
         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 {
@@ -204,7 +202,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:
@@ -227,16 +225,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) {
@@ -251,7 +248,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 @@ int esp_wifi_ap_wps_disable(void)
 static int wifi_ap_wps_start_internal(const unsigned char *pin)
 {
     wifi_mode_t mode = WIFI_MODE_NULL;
+    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

@@ -43,7 +43,8 @@ const char *wps_model_number = CONFIG_IDF_TARGET;
 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);
 #ifdef USE_WPS_TASK
 struct wps_rx_param {
     u8 sa[ETH_ALEN];
@@ -626,6 +627,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;
+}
+
 int wps_process_wps_mX_req(u8 *ubuf, int len, enum wps_process_res *res)
 {
     struct wps_sm *sm = gWpsSm;
@@ -634,6 +681,7 @@ 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) {
@@ -689,35 +737,17 @@ 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);
@@ -1972,7 +2002,7 @@ int wps_check_wifi_mode(void)
 int esp_wifi_wps_enable(const esp_wps_config_t *config)
 {
     int ret = ESP_OK;
-    struct wps_sm *sm = gWpsSm;
+    enum wps_owner owner;
 
     if (esp_wifi_get_user_init_flag_internal() == 0) {
         wpa_printf(MSG_ERROR, "wps enable: wifi not started cannot disable wpsreg");
@@ -1989,8 +2019,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 (sm && os_memcmp(sm->identity, WSC_ID_REGISTRAR, sm->identity_len) == 0) {
+    if (owner != WPS_OWNER_NONE) {
+        if (owner == WPS_OWNER_REGISTRAR) {
             wpa_printf(MSG_ERROR, "wps enable: wpsreg already enabled cannot enable wps enrollee");
             ret = ESP_ERR_WIFI_MODE;
         } else {
@@ -2014,23 +2045,28 @@ int esp_wifi_wps_enable(const esp_wps_config_t *config)
         return ret;
     }
 
-    atomic_store(&s_wps_enabled, true);
+    wps_set_owner(WPS_OWNER_ENROLLEE);
     wpa_printf(MSG_DEBUG, "wifi wps task: prio:%d, stack:%d", 2, WPS_TASK_STACK_SIZE);
     API_MUTEX_GIVE();
     return ret;
 #else
     ret = wifi_wps_enable_internal(config);
     if (ret == ESP_OK) {
-        atomic_store(&s_wps_enabled, true);
+        wps_set_owner(WPS_OWNER_ENROLLEE);
     }
     API_MUTEX_GIVE();
     return ret;
 #endif
 }
 
-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);
 }
 
 int wifi_wps_enable_internal(const esp_wps_config_t *config)
@@ -2051,8 +2087,13 @@ int wifi_wps_enable_internal(const esp_wps_config_t *config)
 
     wpa_printf(MSG_INFO, "wifi_wps_enable");
 
-    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);
 
@@ -2083,26 +2124,27 @@ int esp_wifi_wps_disable(void)
     int ret = 0;
     int wps_status;
     int prev_wps_type;
-    struct wps_sm *wps_sm = gWpsSm;
+    enum wps_owner prev_owner;
     struct wpa_sm *wpa_sm = &gWpaSm;
 
-    if (wps_sm && os_memcmp(wps_sm->identity, WSC_ID_REGISTRAR, wps_sm->identity_len) == 0) {
-        return ESP_ERR_WIFI_MODE;
-    }
-
     API_MUTEX_TAKE();
 
-    if (!atomic_load(&s_wps_enabled)) {
+    prev_owner = wps_get_owner();
+    if (prev_owner == WPS_OWNER_NONE) {
         wpa_printf(MSG_DEBUG, "wps disable: already disabled");
         API_MUTEX_GIVE();
         return ESP_OK;
     }
+    if (prev_owner == WPS_OWNER_REGISTRAR) {
+        API_MUTEX_GIVE();
+        return ESP_ERR_WIFI_MODE;
+    }
 
     wps_status = wps_get_status();
     wpa_printf(MSG_INFO, "wifi_wps_disable");
     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);
 
 #ifdef USE_WPS_TASK
     ret = wps_post_block(SIG_WPS_DISABLE, 0);
@@ -2113,7 +2155,7 @@ int esp_wifi_wps_disable(void)
     if (ESP_OK != ret) {
         wpa_printf(MSG_ERROR, "wps disable: failed to disable wps, ret=%d", ret);
         wps_set_type(prev_wps_type);
-        atomic_store(&s_wps_enabled, true);
+        wps_set_owner(prev_owner);
     }
 
     /* Only disconnect in case of WPS pending */
@@ -2129,17 +2171,25 @@ int esp_wifi_wps_disable(void)
 
 int esp_wifi_wps_start(int timeout_ms)
 {
+    enum wps_owner owner;
+
     if (ESP_OK != wps_check_wifi_mode()) {
         return ESP_ERR_WIFI_MODE;
     }
 
     API_MUTEX_TAKE();
 
-    if (!atomic_load(&s_wps_enabled)) {
+    owner = wps_get_owner();
+    if (owner == WPS_OWNER_NONE) {
         wpa_printf(MSG_ERROR, "wps start: wps not enabled");
         API_MUTEX_GIVE();
         return ESP_ERR_WIFI_WPS_SM;
     }
+    if (owner != WPS_OWNER_ENROLLEE) {
+        wpa_printf(MSG_ERROR, "wps start: wps enrollee not enabled");
+        API_MUTEX_GIVE();
+        return ESP_ERR_WIFI_MODE;
+    }
 
     if (wps_get_type() == WPS_TYPE_DISABLE || (wps_get_status() != WPS_STATUS_DISABLE && wps_get_status() != WPS_STATUS_SCANNING)) {
         API_MUTEX_GIVE();

components/wpa_supplicant/esp_supplicant/src/esp_wps_i.h

@@ -33,6 +33,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 */
@@ -139,7 +145,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);