From efa70bc8e32059a3568fb782f645f889998bb876 Mon Sep 17 00:00:00 2001 From: Liu Zhi Fu Date: Thu, 20 Dec 2018 14:03:11 +0800 Subject: [PATCH] lwip: fix mbox thread-safe issue Fix a mbox free thread-safe issue that can lead to crash in sys_arch_mbox_fetch. --- components/lwip/lwip | 2 +- .../lwip/port/esp32/freertos/sys_arch.c | 4 ++-- .../lwip/port/esp32/include/arch/sys_arch.h | 20 ++++++++++++++++++- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/components/lwip/lwip b/components/lwip/lwip index 0865edf80..e2a24de66 160000 --- a/components/lwip/lwip +++ b/components/lwip/lwip @@ -1 +1 @@ -Subproject commit 0865edf80ad0a96e0eaf2bbedbad350cab248115 +Subproject commit e2a24de6603c896a5ddacac296c0304002376eae diff --git a/components/lwip/port/esp32/freertos/sys_arch.c b/components/lwip/port/esp32/freertos/sys_arch.c index 4270f957b..1168b9e89 100644 --- a/components/lwip/port/esp32/freertos/sys_arch.c +++ b/components/lwip/port/esp32/freertos/sys_arch.c @@ -352,7 +352,6 @@ sys_mbox_free(sys_mbox_t *mbox) uint32_t mbox_message_num = 0; if ( (NULL == mbox) || (NULL == *mbox) ) { - ESP_LOGW(TAG, "WARNING: free null mbox\n"); return; } @@ -390,12 +389,13 @@ sys_mbox_free(sys_mbox_t *mbox) /* For recvmbox or acceptmbox, free them in netconn_free() when all sockets' API are returned */ vQueueDelete((*mbox)->os_mbox); free(*mbox); + *mbox = NULL; } #else vQueueDelete((*mbox)->os_mbox); free(*mbox); -#endif *mbox = NULL; +#endif } /*-----------------------------------------------------------------------------------*/ diff --git a/components/lwip/port/esp32/include/arch/sys_arch.h b/components/lwip/port/esp32/include/arch/sys_arch.h index bfa2ad6ca..9638fdfd6 100644 --- a/components/lwip/port/esp32/include/arch/sys_arch.h +++ b/components/lwip/port/esp32/include/arch/sys_arch.h @@ -62,7 +62,25 @@ typedef struct sys_mbox_s { #endif #define sys_mbox_valid( x ) ( ( ( *x ) == NULL) ? pdFALSE : pdTRUE ) -#define sys_mbox_set_invalid( x ) ( ( *x ) = NULL ) + +/* Define the sys_mbox_set_invalid() to empty to support lock-free mbox in ESP LWIP. + * + * The basic idea about the lock-free mbox is that the mbox should always be valid unless + * no socket APIs are using the socket and the socket is closed. ESP LWIP achieves this by + * following two changes to official LWIP: + * 1. Postpone the deallocation of mbox to netconn_free(), in other words, free the mbox when + * no one is using the socket. + * 2. Define the sys_mbox_set_invalid() to empty if the mbox is not actually freed. + + * The second change is necessary. Consider a common scenario: the application task calls + * recv() to receive packets from the socket, the sys_mbox_valid() returns true. Because there + * is no lock for the mbox, the LWIP CORE can call sys_mbox_set_invalid() to set the mbox at + * anytime and the thread-safe issue may happen. + * + * However, if the sys_mbox_set_invalid() is not called after sys_mbox_free(), e.g. in netconn_alloc(), + * we need to initialize the mbox to invalid explicitly since sys_mbox_set_invalid() now is empty. + */ +#define sys_mbox_set_invalid( x ) #define sys_sem_valid( x ) ( ( ( *x ) == NULL) ? pdFALSE : pdTRUE ) #define sys_sem_set_invalid( x ) ( ( *x ) = NULL )