From 4260ca4b50b7216d18f7bed0857d781da3e3ba65 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 11 Mar 2026 09:26:51 -1000 Subject: [PATCH] [socket] Fix use-after-free in LWIP PCB close/abort path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clear LWIP callbacks (tcp_arg, tcp_recv, tcp_err) before calling tcp_close() or tcp_abort() to prevent use-after-free. After tcp_close(), the PCB remains alive during the TCP close handshake (FIN_WAIT, TIME_WAIT states). If LWIP calls recv/err callbacks during this period and the socket object has already been destroyed, the callback writes to freed memory, corrupting the heap. This was observed as umm_malloc_core crashes on ESP8266 during rapid API client connect/disconnect cycles — the heap free-list got corrupted by a dangling callback writing to a freed LWIPRawImpl object. Extract pcb_detach_abort() and pcb_detach_close() helpers to ensure all close/abort sites consistently clear callbacks first. --- .../components/socket/lwip_raw_tcp_impl.cpp | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index fd1b8a95542..1d9edaebe91 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -138,13 +138,41 @@ static const char *const TAG = "socket.lwip"; #define LWIP_LOG(msg, ...) #endif +// Clear LWIP callbacks and abort a PCB. +// Must be called before destroying the object that `tcp_arg` points to. +// tcp_abort() triggers the err callback synchronously — without clearing +// first, it would call back into a partially-destroyed object, corrupting +// freed memory. +static void pcb_detach_abort(struct tcp_pcb *pcb) { + tcp_arg(pcb, nullptr); + tcp_recv(pcb, nullptr); + tcp_err(pcb, nullptr); + tcp_abort(pcb); +} + +// Clear LWIP callbacks and gracefully close a PCB. +// After tcp_close(), the PCB remains alive during the TCP close handshake +// (FIN_WAIT, TIME_WAIT states). Without clearing callbacks first, LWIP +// would call recv/err on a destroyed socket object, corrupting the heap. +// Returns ERR_OK on success; on failure the PCB is aborted instead. +static err_t pcb_detach_close(struct tcp_pcb *pcb) { + tcp_arg(pcb, nullptr); + tcp_recv(pcb, nullptr); + tcp_err(pcb, nullptr); + err_t err = tcp_close(pcb); + if (err != ERR_OK) { + tcp_abort(pcb); + } + return err; +} + // ---- LWIPRawCommon methods ---- LWIPRawCommon::~LWIPRawCommon() { LWIP_LOCK(); if (this->pcb_ != nullptr) { LWIP_LOG("tcp_abort(%p)", this->pcb_); - tcp_abort(this->pcb_); + pcb_detach_abort(this->pcb_); this->pcb_ = nullptr; } } @@ -222,15 +250,13 @@ int LWIPRawCommon::close() { return -1; } LWIP_LOG("tcp_close(%p)", this->pcb_); - err_t err = tcp_close(this->pcb_); + err_t err = pcb_detach_close(this->pcb_); + this->pcb_ = nullptr; if (err != ERR_OK) { LWIP_LOG(" -> err %d", err); - tcp_abort(this->pcb_); - this->pcb_ = nullptr; errno = err == ERR_MEM ? ENOMEM : EIO; return -1; } - this->pcb_ = nullptr; return 0; } @@ -673,13 +699,10 @@ ssize_t LWIPRawImpl::writev(const struct iovec *iov, int iovcnt) { LWIPRawListenImpl::~LWIPRawListenImpl() { LWIP_LOCK(); // Abort any queued PCBs that were never accepted by the main loop. - // Clear the error callback first — tcp_abort triggers it, and we don't - // want s_queued_err_fn writing to slots during destruction. for (uint8_t i = 0; i < this->accepted_socket_count_; i++) { auto &entry = this->accepted_pcbs_[i]; if (entry.pcb != nullptr) { - tcp_err(entry.pcb, nullptr); - tcp_abort(entry.pcb); + pcb_detach_abort(entry.pcb); entry.pcb = nullptr; } if (entry.rx_buf != nullptr) { @@ -693,7 +716,7 @@ LWIPRawListenImpl::~LWIPRawListenImpl() { // fields that don't exist in the smaller tcp_pcb_listen struct. // Close here and null pcb_ so the base destructor skips tcp_abort. if (this->pcb_ != nullptr) { - tcp_close(this->pcb_); + pcb_detach_close(this->pcb_); this->pcb_ = nullptr; } }