ESP32: use-after-free panic in ECMA-419 TCP tcpReceive when a socket closes with data in flight (lwIP tcpip thread vs XS thread race)

[xymeow]

Environment

• Moddable SDK 8.2.3 (the relevant code in modules/io/socket/lwip/tcp.c is unchanged on the current public branch)
• ESP-IDF 6.0, ESP32-S3 (M5Stack CoreS3), CONFIG_LWIP_TCPIP_CORE_LOCKING not set (IDF default)
• Debug build (mcconfig -d); the device runs an HTTP server (embedded:io listener) serving roughly 0.2–1 request/s from a LAN client, plus occasional outbound HTTP (WAV streaming)

Symptom

Sporadic Guru Meditation Error: Core 1 panic'ed (LoadProhibited) followed by rst:0xc (RTC_SW_CPU_RST). 24 such resets were captured on the serial console over one evening of ordinary traffic. Three panics include full backtraces, all entering through tcpReceive on the lwIP tcpip_thread:

tcpReceive               modules/io/socket/lwip/tcp.c:575
tcp_process_refused_data lwip/src/core/tcp.c:1578
tcp_input                lwip/src/core/tcp_in.c:434
ip4_input / ethernet_input / tcpip_thread

tcpReceive               modules/io/socket/lwip/tcp.c:575
tcp_input                lwip/src/core/tcp_in.c:512
ip4_input ...

fxInNetworkDebugLoop     xs/platforms/esp/xsPlatform.c:122
tcpReceive               modules/io/socket/lwip/tcp.c:559
...

Analysis

tcpReceive runs on the lwIP tcpip_thread. doClose / xs_tcp_destructor run on the XS task and clear the lwIP callbacks with raw tcp_recv/tcp_sent/tcp_err calls and then free the TCP record. (tcp_close itself is marshaled on ESP32 via tcp_close_safe/tcpip_api_call — the callback removal and the free are the unsynchronized steps.) With core locking disabled there is no synchronization between the two threads, so:

1. A segment — or a refused-data redelivery, see the tcp_process_refused_data frame — can enter tcpReceive with arg pointing at a TCP record that the XS thread frees concurrently. The crash sites are the tcp->buffers walk (line 575) and fxInNetworkDebugLoop(tcp->the) (line 559).
2. Refused data widens the window considerably: tcpReceive returns ERR_MEM when c_malloc fails or while fxInNetworkDebugLoop() is true (debug builds), so lwIP parks the pbuf and redelivers it later — for short-lived HTTP connections, frequently after the JS side has already closed the socket.

The existing useCount atomics protect fields of the record but not the lifetime of the arg pointer handed to lwIP callbacks.

Supporting evidence from mitigation

Switching to a release build and lowering the LAN client's polling frequency reduced the crash rate substantially (from ~24/evening) but did not eliminate it — a further rst:0xc occurred after ~17 minutes of release-build uptime, consistent with the direct cross-thread race remaining.

Proposed fix (tested)

The SDK already marshals tcp_close/tcp_write/etc. onto the tcpip thread on ESP32 (modLwipSafe.c's tcpip_api_call idiom) — but callback removal still uses raw tcp_recv/tcp_sent/tcp_err calls from the XS task, immediately before the record is freed. The fix follows the existing idiom:

1. Add tcp_clear_callbacks_safe(pcb) to modLwipSafe.c/h: synchronously (via tcpip_api_call) clear tcp_arg, tcp_recv, tcp_sent, and tcp_err on the tcpip thread. Because tcpip_api_call completes only after the tcpip thread finishes its current message, returning from it guarantees no callback for that pcb is running or can run again — making the subsequent free safe.
2. Use it from removeTCPCallbacks under #if ESP32.
3. Add NULL == arg guards to tcpReceive / tcpSent / tcpError (the cleared tcp_arg makes any residual delivery arrive as NULL; tcpReceive frees the pbuf in that case).

Patch is +44 lines across modules/io/socket/lwip/tcp.c and modules/network/socket/lwip/modLwipSafe.{c,h} (inline below). Running on the affected device now; will report back with soak results. The ECMA-419 UDP glue likely deserves the same treatment for its callback removal, though all captured crashes here were TCP.

Related reports

I could not find an existing issue or PR for this specific crash (searched
tcpReceive, LoadProhibited, use-after-free, and recent commits touching
modules/io/socket/lwip/tcp.c). The closest precedent is the same bug class
in BLE, fixed recently: #1650 "GATTServer crash on late ATT access after
disconnect". This report is the TCP/lwIP analog: a late lwIP callback touching
a record the XS thread has already torn down.

Suggested direction

Marshal callback removal + pcb close + record free onto the tcpip thread (tcpip_callback), or take LOCK_TCPIP_CORE around teardown when core locking is available, or add a lifetime handshake so lwIP callbacks can detect a record that is being torn down.

Happy to provide the full serial capture or test a patch; the crash reproduces within hours on a debug build serving periodic HTTP requests from a LAN poller.

Patch (+44 lines: tcp.c, modLwipSafe.c/h)

diff --git a/modules/io/socket/lwip/tcp.c b/modules/io/socket/lwip/tcp.c
index 0f90023..e1334df 100644
--- a/modules/io/socket/lwip/tcp.c
+++ b/modules/io/socket/lwip/tcp.c
@@ -262,9 +262,18 @@ void xs_tcp_destructor(void *data)
 void removeTCPCallbacks(TCP tcp)
 {
 	if (tcp->skt) {
+#if ESP32
+		// Clearing callbacks from the XS task races the lwIP tcpip task,
+		// which may be inside tcpReceive/tcpSent/tcpError with this record
+		// as arg; the caller frees the record right after. Marshal the clear
+		// onto the tcpip task synchronously: when it returns, no callback
+		// for this pcb is running or can run again.
+		tcp_clear_callbacks_safe(tcp->skt);
+#else
 		tcp_recv(tcp->skt, NULL);
 		tcp_sent(tcp->skt, NULL);
 		tcp_err(tcp->skt, NULL);
+#endif
 	}
 
 	tcp->triggerable &= ~kTCPWritable;
@@ -535,6 +544,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)
@@ -546,6 +558,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
@@ -590,6 +608,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;
 }
diff --git a/modules/network/socket/lwip/modLwipSafe.c b/modules/network/socket/lwip/modLwipSafe.c
index a4cc7fb..dc699dd 100644
--- a/modules/network/socket/lwip/modLwipSafe.c
+++ b/modules/network/socket/lwip/modLwipSafe.c
@@ -169,6 +169,28 @@ void tcp_recved_safe(struct tcp_pcb *tcpPCB, u16_t len)
 	tcpip_callback_with_block(tcp_recved_INLWIP, msg, 1);
 }
 
+static err_t tcp_clear_callbacks_INLWIP(struct tcpip_api_call_data *tcpMsg)
+{
+	LwipMsg msg = (LwipMsg)tcpMsg;
+	if (msg->tcpPCB) {
+		tcp_arg(msg->tcpPCB, NULL);
+		tcp_recv(msg->tcpPCB, NULL);
+		tcp_sent(msg->tcpPCB, NULL);
+		tcp_err(msg->tcpPCB, NULL);
+	}
+	return ERR_OK;
+}
+
+// Synchronous: when this returns, no lwIP callback for this pcb is running
+// or can run again, so the caller may safely free its callback argument.
+void tcp_clear_callbacks_safe(struct tcp_pcb *tcpPCB)
+{
+	LwipMsgRecord msg = {
+		.tcpPCB = tcpPCB,
+	};
+	tcpip_api_call(tcp_clear_callbacks_INLWIP, &msg.call);
+}
+
 static err_t tcp_listen_INLWIP(struct tcpip_api_call_data *tcpMsg)
 {
 	LwipMsg msg = (LwipMsg)tcpMsg;
diff --git a/modules/network/socket/lwip/modLwipSafe.h b/modules/network/socket/lwip/modLwipSafe.h
index 717808d..0f5c94f 100644
--- a/modules/network/socket/lwip/modLwipSafe.h
+++ b/modules/network/socket/lwip/modLwipSafe.h
@@ -52,6 +52,7 @@
 	#include "lwip/raw.h"
 
 	struct tcp_pcb *tcp_new_safe(void);
+	void tcp_clear_callbacks_safe(struct tcp_pcb *tcpPCB);
 	err_t tcp_connect_safe(struct tcp_pcb *tcpPCB, const ip_addr_t *ipaddr, u16_t port, tcp_connected_fn connected);
 	u8_t pbuf_free_safe(struct pbuf *p);
 	err_t tcp_bind_safe(struct tcp_pcb *tcpPCB, const ip_addr_t *ipaddr, u16_t port);

---

Edited: corrected the tcp_close_safe description (it is marshaled on ESP32; the unsynchronized step is callback removal), updated the mitigation results (reduced, not eliminated), and added the tested patch.

[phoddie]

Thank you for the report.

I'm not convinced that introducing tcp_clear_callbacks_safe() is necessary. The lwip functions to set the callbacks can't really race – they just set the callback.

void
tcp_recv(struct tcp_pcb *pcb, tcp_recv_fn recv)
{
  LWIP_ASSERT_CORE_LOCKED();
  if (pcb != NULL) {
    LWIP_ASSERT("invalid socket state for recv callback", pcb->state != LISTEN);
    pcb->recv = recv;
  }
}

Some versions of lwip are even simpler:

void
tcp_recv(struct tcp_pcb *pcb, tcp_recv_fn recv)
{
  pcb->recv = recv;
}

The part of the change that seems most relevant is that removeTCPCallbacks() now also clears the argument to the callbacks using tcp_arg(). This allows the callbacks to determine if they are invoked after they expect by checking for arg of NULL.

Since you have a clearly reproducible case (and I don't!), would you please try again using tcp_clear_callbacks() in place of tcp_clear_callbacks_safe() but still calling tcp_arg() before clearing the callbacks in removeTCPCallbacks()? Of course the checks for arg being NULL are still appropriate. Thank you.

[xymeow]

Thanks — testing your variant now: tcp_arg(pcb, NULL) first, then plain tcp_recv/tcp_sent/tcp_err(pcb, NULL) in removeTCPCallbacks(), no marshaling, with the NULL-arg guards in tcpReceive/tcpSent/tcpError. Just flashed onto the reproducing device; I'll report soak results here.

Two findings since filing that are relevant to interpreting those results:

1. There is a second, independent race behind the same panic signature. With only the teardown change applied, the device kept crashing at the walker->next tail walk — which is exactly where both line-575 backtraces in the report point. The receive-buffer list is mutated from both tasks with no synchronization:

• lwIP task, tcpReceive: walks to the tail of tcp->buffers and appends;
• XS task, xs_tcp_read: pops the head node and c_frees it.

When the reader pops-and-frees the node the walker is standing on, the walk dereferences freed memory. Wrapping just the pointer surgery on both sides in builtinCriticalSection (heap operations kept outside) stopped those crashes. That change stays in place while testing the teardown variant, so the soak isolates the teardown question specifically.

2. Soak-data hygiene. Uncaught app-level exceptions restart release builds silently (fxAbort → c_exit), which is indistinguishable from a crash in an uptime log. To keep the data clean, this build also defines MODDEF_XS_ABORTHOOK with a hook that persists the abort reason before restart — so SDK-race candidates (hardware panic signatures) and app-level restarts are now separable.

Will follow up with results for the non-marshaled variant.

[xymeow]

Results are in, and your suggestion looks right — thank you!

Tested exactly as you described: in removeTCPCallbacks(), tcp_arg(pcb, NULL) first, then plain tcp_recv/tcp_sent/tcp_err(pcb, NULL) — no marshaling — with the NULL-arg guards in tcpReceive/tcpSent/tcpError. The buffer-list critical sections from my previous comment were unchanged, so this run isolates the teardown variant.

Result: 2.1 hours of continuous uptime and counting on the same device and traffic profile that previously crashed every 15–25 minutes (24 resets in one evening at the worst). Zero panics, zero resets, and — via the abort-hook instrumentation — zero app-level restarts either, so the clean run isn't masking anything.

So the simpler fix appears sufficient: clearing the callback argument first gives late callbacks a safe NULL to observe, and the pointer stores don't need the tcpip-thread round trip. I'll keep it soaking and will report back if anything recurs over the next days.

Happy to open a PR with the two changes (arg-first callback clearing + NULL guards, and the builtinCriticalSection protection around the tcp->buffers pointer surgery) — or leave them to you if you'd rather land it differently. Either way, thanks for the quick and precise guidance.

[phoddie]

Thanks for trying that.

If you'd open a PR, that would be helpful. For us to land it, we will need a completed Contributor Agreement. That should just take a couple minutes.

[xymeow]

Thank you! The completed individual CLA has been emailed to info@moddable.com, and the PR is open: #1656.

It contains exactly the two changes discussed above: arg-first callback clearing in removeTCPCallbacks() with NULL-argument guards in the delivery callbacks, and builtinCriticalSection around the tcp->buffers pointer surgery. The reproducing device continues to soak on this exact build — I'll follow up here with multi-day results.

[xymeow]

Following up with an update, and a correction to what I told you earlier.

I need to walk back "the simpler arg-first variant looks sufficient." That 2.1-hour clean run was real, but it was on a build that handled HTTP connections one at a time. I've since reworked my app to serve them concurrently, which pushed the connection open/close rate up a lot, and the tcpReceive LoadProhibited came back — now every 25–30 minutes under even light polling.

The fault looks different this time, though. tcp (the callback arg) is valid; the crash is in the tcp->buffers walk and EXCVADDR is a poison value rather than a small struct offset:

Guru Meditation Error: Core 1 panic'ed (LoadProhibited)
EXCVADDR: 0xe2f61b44
PC -> tcpReceive   modules/io/socket/lwip/tcp.c
      tcp_input / ip4_input / ethernet_input
      tcpip_thread

So this reads like a receive-buffer node that got c_freed while still linked into tcp->buffers, rather than the socket record going away under the callback. That list is touched from both tasks — tcpReceive appends at the tail (tcpip thread), xs_tcp_read pops the head and frees it (XS thread), and the destructor drains it. I wrapped the pointer surgery on each side in builtinCriticalSection, and also tried marshaling the teardown clear, but the poison UAF still recurs — so I'm clearly missing something about how a node ends up freed while it's still reachable from the walk. This is ESP32-S3, dual core, CONFIG_LWIP_TCPIP_CORE_LOCKING off.

Rather than keep patching blind and risk making it worse, I wanted to ask before going further: is there an intended ownership model for the tcp->buffers node lifetime across the two tasks that I should be matching? Happy to test anything you suggest on the unit that reproduces this reliably. I've left the PR untouched for now, since the picture has clearly moved since I opened it.

One aside, much lower priority: while stress-testing I could also trip the LWIP_ASSERT(pcb->state != LISTEN) in tcp_err from the accept path (the //@@ also install error handler spot in listenerAccept), but only by opening a batch of connections and RSTing them all at once — which normal traffic doesn't do. Flagging it in case it's useful; it isn't what's biting in practice.

Thanks again for staying with this.

[phoddie]

Stress testing is an epic adventure.

tcp->buffers is a linked list of received data buffers delivered by lwip but not yet consumed by JavaScript. It is the responsibility of tcp.c to free those once they are read by JavaScript or the instance is closed with them unread. That list is currently accessed from two different tasks and so conventional wisdom is that it should protected by a mutex. That said.. since the lwip task only appends and the main task only pops the first element, a mutex might be unnecessary.

And adding a mutex doesn't solve the crash you see. If I understand well, you are seeing tcpReceive called with a valid tcp pointer but invalid tcp->buffers. That means that reading a field of tcp->buffers fails?

It isn't clear to me if the tcpReceive crash happens after starting to close the TCP socket or while it is simply active. From the rest of the conversation, I assume it is while close is in progress.

this reads like a receive-buffer node that got c_freed

The only way I could see that is if the tcpReceive callback is invoked after xs_tcp_destructor runs. That would be very unexpected. But since arg is non-NULL, that can't be the case?

The listener assert seems likely unrelated, unless it is somehow corrupting lwip internals which results in spraying memory (which could mess up tcp->buffers somehow... but that's a lot of speculation and bad luck). Is there something that can be done outside of lwip address that?