[socket] Fix use-after-free in LWIP PCB close/abort path

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.
This commit is contained in:
J. Nick Koston
2026-03-11 09:26:51 -10:00
parent 928f6f1866
commit 4260ca4b50
+33 -10
View File
@@ -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;
}
}