fix(esp_netif): Fix races in netif object locking

components/esp_netif/esp_netif_objects.c

@@ -1,16 +1,8 @@
-// Copyright 2015-2019 Espressif Systems (Shanghai) PTE LTD
+/*
-//
+ * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
-// Licensed under the Apache License, Version 2.0 (the "License");
+ *
-// you may not use this file except in compliance with the License.
+ * SPDX-License-Identifier: Apache-2.0
-// You may obtain a copy of the License at
+ */
-
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
 
 #include "esp_netif.h"
 #include "sys/queue.h"
@@ -40,25 +32,41 @@ static xSemaphoreHandle  s_list_lock = NULL;
 
 ESP_EVENT_DEFINE_BASE(IP_EVENT);
 
+esp_err_t esp_netif_objects_init(void)
+{
+    if (s_list_lock != NULL) {
+        // already initialized
+        return ESP_OK;
+    }
+    s_list_lock = xSemaphoreCreateMutex();
+    if (s_list_lock == NULL) {
+        return ESP_ERR_NO_MEM;
+    }
+    return ESP_OK;
+}
+
+void esp_netif_objects_deinit(void)
+{
+    vSemaphoreDelete(s_list_lock);
+    s_list_lock = NULL;
+}
+
 esp_err_t esp_netif_list_lock(void)
 {
-    if (s_list_lock == NULL) {
+    if (s_list_lock) {
-        s_list_lock = xSemaphoreCreateMutex();
+        xSemaphoreTake(s_list_lock, portMAX_DELAY);
-        if (s_list_lock == NULL) {
+    } else {
-            return ESP_ERR_NO_MEM;
+        ESP_LOGD(TAG, "%s list not locked (s_list_lock not initialized)", __func__);
-        }
     }
-    xSemaphoreTake(s_list_lock, portMAX_DELAY);
     return ESP_OK;
 }
 
 void esp_netif_list_unlock(void)
 {
-    assert(s_list_lock);
+    if (s_list_lock) {
-    xSemaphoreGive(s_list_lock);
+        xSemaphoreGive(s_list_lock);
-    if (s_esp_netif_counter == 0) {
+    } else {
-        vQueueDelete(s_list_lock);
+        ESP_LOGD(TAG, "%s list not unlocked (s_list_lock not initialized)", __func__);
-        s_list_lock = NULL;
     }
 }
 

components/esp_netif/lwip/esp_netif_lwip.c

@@ -340,6 +340,10 @@ static void tcpip_init_done(void *arg)
 
 esp_err_t esp_netif_init(void)
 {
+    if (esp_netif_objects_init() != ESP_OK) {
+        ESP_LOGE(TAG, "esp_netif_objects_init() failed");
+        return ESP_FAIL;
+    }
     if (!sys_thread_tcpip(LWIP_CORE_IS_TCPIP_INITIALIZED)) {
 #if CONFIG_LWIP_HOOK_TCP_ISN_DEFAULT
         uint8_t rand_buf[16];
@@ -392,6 +396,11 @@ esp_err_t esp_netif_init(void)
 
 esp_err_t esp_netif_deinit(void)
 {
+    /* esp_netif_deinit() is not supported (as lwIP deinit isn't suported either)
+     * Once it's supported, we need to de-initialize:
+     * - netif objects calling esp_netif_objects_deinit()
+     * - other lwIP specific objects (see the comment after tcpip_initialized)
+     */
     if (sys_thread_tcpip(LWIP_CORE_IS_TCPIP_INITIALIZED)) {
         /* deinit of LwIP not supported:
          * do not deinit semaphores and states,
@@ -566,8 +575,6 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config)
     lwip_netif->state = esp_netif;
     esp_netif->lwip_netif = lwip_netif;
 
-    esp_netif_add_to_list(esp_netif);
-
     // Configure the created object with provided configuration
     esp_err_t ret =  esp_netif_init_configuration(esp_netif, esp_netif_config);
     if (ret != ESP_OK) {
@@ -576,6 +583,8 @@ esp_netif_t *esp_netif_new(const esp_netif_config_t *esp_netif_config)
         return NULL;
     }
 
+    esp_netif_add_to_list(esp_netif);
+
     return esp_netif;
 }
 

components/esp_netif/private_include/esp_netif_private.h

@@ -201,4 +201,17 @@ esp_err_t esp_netif_add_ip6_address(esp_netif_t *esp_netif, const ip_event_add_i
  */
 esp_err_t esp_netif_remove_ip6_address(esp_netif_t *esp_netif, const esp_ip6_addr_t *addr);
 
+/**
+ * @brief Initialize netif objects for handling lists of interfaces one esp_netif level
+ *
+ * @return esp_err_t ESP_OK on success
+ */
+esp_err_t esp_netif_objects_init(void);
+
+/**
+ * @brief Deinitialize netif objects
+ *
+ */
+void esp_netif_objects_deinit(void);
+
 #endif //_ESP_NETIF_PRIVATE_H_