From a68ebe1877ee1f533bd7e03f1b4033588bcd2525 Mon Sep 17 00:00:00 2001 From: Xiaoyi He Date: Mon, 13 Jul 2026 15:30:17 -0700 Subject: [PATCH] ESP32 419 TCP: fix two lwip/XS task races that crash under connection churn Fixes two use-after-free races behind the LoadProhibited panics reported in #1655, both between the lwIP tcpip task and the XS task: 1. Socket teardown: removeTCPCallbacks now clears the callback argument (tcp_arg) before clearing the callbacks, and tcpReceive/tcpSent/ tcpError bail out on a NULL argument, so a late delivery - including lwIP refused-data redelivery - cannot touch a freed TCP record. 2. Receive-buffer list: tcpReceive (lwIP task) walks to the tail of tcp->buffers to append while xs_tcp_read (XS task) pops and frees head nodes; the unsynchronized tail walk could traverse a node mid-free. Both sides now do their pointer surgery inside builtinCriticalSection, with heap operations kept outside. Verified on ESP32-S3 (M5Stack CoreS3) under the reproducing workload - an HTTP server polled by a LAN client, previously crashing every 15-25 minutes: hours of clean uptime with both fixes applied, instrumented to exclude unrelated app-level restarts. Co-Authored-By: Claude Fable 5 --- modules/io/socket/lwip/tcp.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/modules/io/socket/lwip/tcp.c b/modules/io/socket/lwip/tcp.c index fcb150fd9..e2a8bbb59 100644 --- a/modules/io/socket/lwip/tcp.c +++ b/modules/io/socket/lwip/tcp.c @@ -259,6 +259,10 @@ void xs_tcp_destructor(void *data) void removeTCPCallbacks(TCP tcp) { if (tcp->skt) { + // clear the argument first so a late callback observes NULL + // (the lwIP tcpip task can deliver segments - including refused-data + // redelivery - after the XS task tears this socket down) + tcp_arg(tcp->skt, NULL); tcp_recv(tcp->skt, NULL); tcp_sent(tcp->skt, NULL); tcp_err(tcp->skt, NULL); @@ -355,7 +359,9 @@ void xs_tcp_read(xsMachine *the) buffer->fragment = fragment->next; buffer->fragmentOffset = 0; if (NULL == buffer->fragment) { + builtinCriticalSectionBegin(); tcp->buffers = buffer->next; + builtinCriticalSectionEnd(); tcp_recved_safe(tcp->skt, buffer->pb->tot_len); pbuf_free_safe(buffer->pb); c_free(buffer); @@ -532,6 +538,9 @@ void tcpError(void *arg, err_t err) { TCP tcp = arg; + if (NULL == tcp) // callbacks cleared during teardown + return; + removeTCPCallbacks(tcp); tcp->skt = NULL; // "pcb is already freed when this callback is called" if (kTCPError & tcp->triggerable) @@ -543,6 +552,12 @@ err_t tcpReceive(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err) TCP tcp = arg; TCPBuffer buffer; + if (NULL == tcp) { // callbacks cleared during teardown + if (pb) + pbuf_free(pb); + return ERR_OK; + } + if ((NULL == pb) || (ERR_OK != err)) { //@@ when is err set here? removeTCPCallbacks(tcp); #if ESP32 @@ -569,6 +584,10 @@ err_t tcpReceive(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err) modInstrumentationAdjust(NetworkBytesRead, buffer->bytes); + // xs_tcp_read pops and frees head nodes on the XS task while this runs + // on the lwIP task; guard the pointer surgery so the tail walk cannot + // traverse a node being detached and freed + builtinCriticalSectionBegin(); if (tcp->buffers) { TCPBuffer walker; @@ -578,6 +597,7 @@ err_t tcpReceive(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err) } else tcp->buffers = buffer; + builtinCriticalSectionEnd(); if (tcp->triggerable & kTCPReadable) tcpTrigger(tcp, kTCPReadable); @@ -587,6 +607,9 @@ err_t tcpReceive(void *arg, struct tcp_pcb *pcb, struct pbuf *pb, err_t err) err_t tcpSent(void *arg, struct tcp_pcb *pcb, u16_t len) { + if (NULL == arg) // callbacks cleared during teardown + return ERR_OK; + tcpTrigger((TCP)arg, kTCPWritable); return ERR_OK; }