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
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// 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.
+/*
+ * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
 
 #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) {
-        s_list_lock = xSemaphoreCreateMutex();
-        if (s_list_lock == NULL) {
-            return ESP_ERR_NO_MEM;
-        }
+    if (s_list_lock) {
+        xSemaphoreTake(s_list_lock, portMAX_DELAY);
+    } else {
+        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);
-    xSemaphoreGive(s_list_lock);
-    if (s_esp_netif_counter == 0) {
-        vQueueDelete(s_list_lock);
-        s_list_lock = NULL;
+    if (s_list_lock) {
+        xSemaphoreGive(s_list_lock);
+    } else {
+        ESP_LOGD(TAG, "%s list not unlocked (s_list_lock not initialized)", __func__);
     }
 }
 

components/esp_netif/lwip/esp_netif_lwip.c

@@ -312,6 +312,10 @@ void* esp_netif_get_netif_impl(esp_netif_t *esp_netif)
 
 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 (tcpip_initialized == false) {
         tcpip_initialized = true;
 #if CONFIG_LWIP_HOOK_TCP_ISN_DEFAULT
@@ -350,6 +354,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 (tcpip_initialized == true) {
         /* deinit of LwIP not supported:
          * do not deinit semaphores and states,
@@ -515,8 +524,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) {
@@ -525,6 +532,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

@@ -145,4 +145,17 @@ void esp_netif_list_unlock(void);
  */
 bool esp_netif_is_netif_listed(esp_netif_t *esp_netif);
 
+/**
+ * @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_