esp_modem: spinlock_acquire assert (use-after-free) when tearing down / switching modes while RX data is in flight

[thiker5]

Answers checklist.

• I have read the documentation for esp-protocols components and the issue is not addressed there.
• I have updated my esp-protocols branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

Environment

• Component: esp_modem (~v1.4.2)
• Target: ESP32 (dual-core)
• Transport: UART terminal; reproducible both in plain command/data mode and CMUX

Summary

Destroying the DTE/DCE (or calling set_mode() / exit_data() / exit_cmux())
while the modem link is still receiving data can crash with a FreeRTOS spinlock
corruption assert. The crash is a use-after-free of a terminal callback (or the
mutex it takes) by the terminal RX task.

Backtrace

assert failed: spinlock_acquire spinlock.h:123 ((result == SPINLOCK_FREE) == (lock->count == 0))
0x400818ef: panic_abort at panic.c:407
0x4008b815: esp_system_abort at system_api.c:112
0x40092645: __assert_func at assert.c:85
0x4008ea8d: spinlock_acquire at spinlock.h:123
(inlined by) vPortEnterCritical at port.c:448
0x4008c6d9: xQueueSemaphoreTake at queue.c:1466
0x4008c848: xQueueTakeMutexRecursive at queue.c:687
0x400f756a: esp_modem::Lock::lock() at esp_modem_primitives_freertos.cpp:33
0x400fa9a1: esp_modem::Scoped<esp_modem::Lock>::Scoped() at esp_modem_primitives.hpp:53
(inlined by) operator() at esp_modem_dte.cpp:67 // the read callback taking command_cb.line_lock
0x400f7c41: esp_modem::UartTerminal::task() at esp_modem_uart.cpp:167
0x400f7c51: esp_modem::UartTerminal::s_task(void*) at esp_modem_uart.cpp:74
0x4008e8e6: vPortTaskWrapper at port.c:168

Root cause

Terminal callbacks are mutated without any synchronization against the RX task
that invokes them:

• Terminal::set_read_cb()/set_error_cb() and CMux::set_read_cb() reassign a
  std::function (or read_cb[]) with no lock.
• set_mode()/set_command_callbacks() swap and rebind these during DATA/CMUX
  transitions; ~DTE lets command_cb (which owns line_lock) be destroyed
  while the RX task is still running.
• internal_lock (held during set_mode) does not protect this, because the
  RX task (UartTerminal::task()) never takes internal_lock.

So the RX task can be executing a callback whose std::function target is being
destroyed, or whose captured this/command_cb.line_lock has already been freed
— it then locks a freed/garbage mutex and the spinlock consistency assert fires.

Separately, stop() is effectively a no-op for loop exit (it sets TASK_STOP
but the loop checks TASK_START), so the RX task is always force-deleted via
vTaskDelete() — potentially mid-callback or inside the UART driver — and
FdTerminal additionally has a self-delete vs owner-delete double-delete window.

Reproduction

1. Bring up PPP/CMUX so the modem is actively pushing RX data.
2. Tear down the DCE/DTE (or call exit_data() / set_mode(COMMAND_MODE))
   while data is still arriving.
3. Intermittent crash with the assert above (timing/SMP dependent).
   Note: the below will have the modem crash in no time
   In file : esp_modem_uart.cpp/get_event --> change the timeout to 1
   *Initialize via
   cell_netif= esp_netif_new(&config)
   dce = esp_modem_new(...)
   esp_event_handler_instance_register(...)

Call long AT command (AT+COPS? for example) with immediate return (timeout 1 ms) then
*Destroy
esp_event_handler_instance_unregister(....)
sct_esp_modem_destroy(dce);
esp_netif_destroy(cell_netif);
cell_netif= NULL;
dce = NULL;

Proposed fix

• Serialize callback (re)assignment against invocation with a recursive
  callback lock (per terminal; a dedicated one in CMux kept separate from its
  state lock so a blocking RX upcall can't deadlock write()).
• Clear/stop callbacks in ~DTE()/~CMux() before the captured state is
  destroyed.
• Make stop() graceful and synchronous (a TASK_STOPPED handshake), so the
  RX task is quiesced before deletion instead of force-deleted.

Happy to open a PR with these changes.