From 9d3c8f0532a4000ba7457828b3eba015dd256b95 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Mon, 9 Dec 2024 18:35:29 +0100 Subject: [PATCH 1/2] fix(lwip): Fix ping session calling thread unsafe API Closes https://github.com/espressif/esp-idf/issues/14982 --- components/lwip/apps/ping/ping_sock.c | 34 +++++++++++++++++-- .../icmp_echo/main/echo_example_main.c | 8 +++-- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/components/lwip/apps/ping/ping_sock.c b/components/lwip/apps/ping/ping_sock.c index bb1703558c..d1014a92d1 100644 --- a/components/lwip/apps/ping/ping_sock.c +++ b/components/lwip/apps/ping/ping_sock.c @@ -7,6 +7,7 @@ #include #include #include +#include #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "lwip/opt.h" @@ -25,6 +26,35 @@ #include "ping/ping_sock.h" #include "esp_check.h" +#ifndef CONFIG_LWIP_NETIF_API + +#include "lwip/priv/tcpip_priv.h" +// If POSIX NETIF_API not enabled, we need to supply the implementation of if_indextoname() +// using tcpip_callback() + +struct tcpip_netif_name { + struct tcpip_api_call_data call; + u8_t ifindex; + char *ifname; + err_t err; +}; + +static void do_netif_index_to_name(void *ctx) +{ + struct tcpip_netif_name *params = ctx; + params->err = netif_index_to_name(params->ifindex, params->ifname) ? ERR_OK : ERR_IF; +} + +char *if_indextoname(unsigned int ifindex, char *ifname) +{ + struct tcpip_netif_name params = { .ifindex = ifindex, .ifname = ifname }; + if (tcpip_callback(do_netif_index_to_name, ¶ms) != ERR_OK || params.err != ERR_OK) { + return NULL; + } + return ifname; +} +#endif // CONFIG_LWIP_NETIF_API == 0 + const static char *TAG = "ping_sock"; #define PING_TIME_DIFF_MS(_end, _start) ((uint32_t)(((_end).tv_sec - (_start).tv_sec) * 1000 + \ @@ -266,8 +296,8 @@ esp_err_t esp_ping_new_session(const esp_ping_config_t *config, const esp_ping_c /* set if index */ if(config->interface) { struct ifreq iface; - if(netif_index_to_name(config->interface, iface.ifr_name) == NULL) { - ESP_LOGE(TAG, "fail to find interface name with netif index %d", config->interface); + if (if_indextoname(config->interface, iface.ifr_name) == NULL) { + ESP_LOGE(TAG, "fail to find interface name with netif index %" PRIu32, config->interface); goto err; } if(setsockopt(ep->sock, SOL_SOCKET, SO_BINDTODEVICE, &iface, sizeof(iface)) != 0) { diff --git a/examples/protocols/icmp_echo/main/echo_example_main.c b/examples/protocols/icmp_echo/main/echo_example_main.c index d38134ce88..d04999da42 100644 --- a/examples/protocols/icmp_echo/main/echo_example_main.c +++ b/examples/protocols/icmp_echo/main/echo_example_main.c @@ -19,6 +19,9 @@ #include "argtable3/argtable3.h" #include "protocol_examples_common.h" #include "ping/ping_sock.h" +#include "esp_check.h" + +const static char *TAG = "echo_example"; static void cmd_ping_on_ping_success(esp_ping_handle_t hdl, void *args) { @@ -148,9 +151,8 @@ static int do_ping_cmd(int argc, char **argv) .on_ping_end = cmd_ping_on_ping_end }; esp_ping_handle_t ping; - esp_ping_new_session(&config, &cbs, &ping); - esp_ping_start(ping); - + ESP_RETURN_ON_FALSE(esp_ping_new_session(&config, &cbs, &ping) == ESP_OK, -1, TAG, "esp_ping_new_session failed"); + ESP_RETURN_ON_FALSE(esp_ping_start(ping) == ESP_OK, -1, TAG, "esp_ping_start() failed"); return 0; } From 8b0c48818442272d7288d98164bddfb268251acc Mon Sep 17 00:00:00 2001 From: David Cermak Date: Mon, 20 Jan 2025 10:43:18 +0100 Subject: [PATCH 2/2] fix(lwip): Fix potential data-race in ping tcpip callback Need to use tcpip_api_call() instead of tcpip_callback(), since the former waits for the tcpip task to complete and thus prevents potential data races with subsequent TCP/IP tasks. --- components/lwip/apps/ping/ping_sock.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/components/lwip/apps/ping/ping_sock.c b/components/lwip/apps/ping/ping_sock.c index d1014a92d1..2162b13a81 100644 --- a/components/lwip/apps/ping/ping_sock.c +++ b/components/lwip/apps/ping/ping_sock.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -27,28 +27,26 @@ #include "esp_check.h" #ifndef CONFIG_LWIP_NETIF_API - -#include "lwip/priv/tcpip_priv.h" // If POSIX NETIF_API not enabled, we need to supply the implementation of if_indextoname() -// using tcpip_callback() +// using tcpip_api_call() +#include "lwip/priv/tcpip_priv.h" struct tcpip_netif_name { struct tcpip_api_call_data call; u8_t ifindex; char *ifname; - err_t err; }; -static void do_netif_index_to_name(void *ctx) +static err_t do_netif_index_to_name(struct tcpip_api_call_data *msg) { - struct tcpip_netif_name *params = ctx; - params->err = netif_index_to_name(params->ifindex, params->ifname) ? ERR_OK : ERR_IF; + struct tcpip_netif_name *params = __containerof(msg, struct tcpip_netif_name, call); + return netif_index_to_name(params->ifindex, params->ifname) ? ERR_OK : ERR_IF; } char *if_indextoname(unsigned int ifindex, char *ifname) { struct tcpip_netif_name params = { .ifindex = ifindex, .ifname = ifname }; - if (tcpip_callback(do_netif_index_to_name, ¶ms) != ERR_OK || params.err != ERR_OK) { + if (tcpip_api_call(do_netif_index_to_name, ¶ms.call) != ERR_OK) { return NULL; } return ifname;