From ad25b64faba075de4df1ff4c74fd1475e29e5683 Mon Sep 17 00:00:00 2001 From: jake champion Date: Fri, 27 Feb 2026 10:55:40 +0000 Subject: [PATCH 1/4] fix cancel/accept race in NetAccept that causes EBADF abort `NetAcceptAction::cancel()` closes the server socket before setting the cancelled flag In the window between close and flag set, `net_accept()` can get `EBADF` from `accept4()`, see `cancelled=false`, and dispatch `EVENT_ERROR` to handlers that don't expect it now we check the atomic server pointer in all three accept paths before dispatching `EVENT_ERROR` --- src/iocore/net/UnixNetAccept.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/iocore/net/UnixNetAccept.cc b/src/iocore/net/UnixNetAccept.cc index 66e73096fa2..0e72a681c7c 100644 --- a/src/iocore/net/UnixNetAccept.cc +++ b/src/iocore/net/UnixNetAccept.cc @@ -112,7 +112,7 @@ net_accept(NetAccept *na, void *ep, bool blockable) if (res == -EAGAIN || res == -ECONNABORTED || res == -EPIPE) { goto Ldone; } - if (na->server.sock.is_ok() && !na->action_->cancelled) { + if (na->action_->server.load(std::memory_order_acquire) != nullptr && na->server.sock.is_ok() && !na->action_->cancelled) { if (!blockable) { na->action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); } else { @@ -387,7 +387,7 @@ NetAccept::do_blocking_accept(EThread *t) case -1: [[fallthrough]]; default: - if (!action_->cancelled) { + if (action_->server.load(std::memory_order_acquire) != nullptr && !action_->cancelled) { SCOPED_MUTEX_LOCK(lock, action_->mutex ? action_->mutex : t->mutex, t); action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); Warning("accept thread received fatal error: errno = %d", errno); @@ -580,7 +580,7 @@ NetAccept::acceptFastEvent(int event, void *ep) check_transient_accept_error(res); goto Ldone; } - if (!action_->cancelled) { + if (action_->server.load(std::memory_order_acquire) != nullptr && !action_->cancelled) { action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); } goto Lerror; From ae2fd6700057f540626a68f9726449744b37f483 Mon Sep 17 00:00:00 2001 From: jake champion Date: Thu, 9 Jul 2026 14:47:18 +0100 Subject: [PATCH 2/4] NetAccept: drop redundant is_ok() check in net_accept path --- src/iocore/net/UnixNetAccept.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iocore/net/UnixNetAccept.cc b/src/iocore/net/UnixNetAccept.cc index 0e72a681c7c..b6e28d21f7d 100644 --- a/src/iocore/net/UnixNetAccept.cc +++ b/src/iocore/net/UnixNetAccept.cc @@ -112,7 +112,7 @@ net_accept(NetAccept *na, void *ep, bool blockable) if (res == -EAGAIN || res == -ECONNABORTED || res == -EPIPE) { goto Ldone; } - if (na->action_->server.load(std::memory_order_acquire) != nullptr && na->server.sock.is_ok() && !na->action_->cancelled) { + if (na->action_->server.load(std::memory_order_acquire) != nullptr && !na->action_->cancelled) { if (!blockable) { na->action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); } else { From f041676b844354cc7542f2eb6d6135598ac228ef Mon Sep 17 00:00:00 2001 From: jake champion Date: Thu, 17 Sep 2026 14:23:57 +0100 Subject: [PATCH 3/4] NetAccept: drop racy cancelled reads from accept error guards --- src/iocore/net/UnixNetAccept.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/iocore/net/UnixNetAccept.cc b/src/iocore/net/UnixNetAccept.cc index b6e28d21f7d..b23f617f4fb 100644 --- a/src/iocore/net/UnixNetAccept.cc +++ b/src/iocore/net/UnixNetAccept.cc @@ -112,7 +112,7 @@ net_accept(NetAccept *na, void *ep, bool blockable) if (res == -EAGAIN || res == -ECONNABORTED || res == -EPIPE) { goto Ldone; } - if (na->action_->server.load(std::memory_order_acquire) != nullptr && !na->action_->cancelled) { + if (na->action_->server.load(std::memory_order_acquire) != nullptr) { if (!blockable) { na->action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); } else { @@ -387,7 +387,7 @@ NetAccept::do_blocking_accept(EThread *t) case -1: [[fallthrough]]; default: - if (action_->server.load(std::memory_order_acquire) != nullptr && !action_->cancelled) { + if (action_->server.load(std::memory_order_acquire) != nullptr) { SCOPED_MUTEX_LOCK(lock, action_->mutex ? action_->mutex : t->mutex, t); action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); Warning("accept thread received fatal error: errno = %d", errno); @@ -580,7 +580,7 @@ NetAccept::acceptFastEvent(int event, void *ep) check_transient_accept_error(res); goto Ldone; } - if (action_->server.load(std::memory_order_acquire) != nullptr && !action_->cancelled) { + if (action_->server.load(std::memory_order_acquire) != nullptr) { action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); } goto Lerror; From b5091787cf3cfc3ba5e6e1c78a763980fca498fc Mon Sep 17 00:00:00 2001 From: jake champion Date: Thu, 17 Sep 2026 14:33:41 +0100 Subject: [PATCH 4/4] NetAccept: hide the accept action's server pointer behind an accessor --- src/iocore/net/P_NetAccept.h | 30 +++++++++++++++++++++++++----- src/iocore/net/UnixNetAccept.cc | 6 +++--- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/iocore/net/P_NetAccept.h b/src/iocore/net/P_NetAccept.h index 89650e34fa6..b3ca8e654ee 100644 --- a/src/iocore/net/P_NetAccept.h +++ b/src/iocore/net/P_NetAccept.h @@ -62,23 +62,38 @@ AcceptFunction net_accept; class UnixNetVConnection; struct NetAcceptAction : public Action, public RefCountObjInHeap { - std::atomic server{nullptr}; - NetAcceptAction(Continuation *cont, Server *s) { continuation = cont; if (cont != nullptr) { mutex = cont->mutex; } - server.store(s, std::memory_order_release); + _server.store(s, std::memory_order_release); + } + + /** Whether this action still owns an open listening socket. + + Accept paths must consult this before dispatching EVENT_ERROR. It is + cleared by cancel() before Action::cancel() sets @c cancelled, so it goes + false no later than the cancellation the continuation is aware of, and it + is safe to read from any thread. Reading @c cancelled instead is both a + data race on a plain bool and too late: accept() reports EBADF as soon as + the socket closes, which is before @c cancelled is set. + + */ + bool + is_listening() const + { + return _server.load(std::memory_order_acquire) != nullptr; } void cancel(Continuation *cont = nullptr) override { // Use atomic exchange so only one thread closes the server, preventing - // use-after-free races between cancel() and acceptEvent() cleanup. - Server *s = server.exchange(nullptr, std::memory_order_acq_rel); + // use-after-free races between cancel() and acceptEvent() cleanup. This + // must stay ahead of Action::cancel(), see is_listening(). + Server *s = _server.exchange(nullptr, std::memory_order_acq_rel); if (s != nullptr) { s->close(); } @@ -92,6 +107,11 @@ struct NetAcceptAction : public Action, public RefCountObjInHeap { static DbgCtl dbg_ctl{"net_accept"}; Dbg(dbg_ctl, "NetAcceptAction dying"); } + +private: + /// Cleared exactly once, by cancel(). Private so the ordering above is the + /// only way this transitions to null. + std::atomic _server{nullptr}; }; // diff --git a/src/iocore/net/UnixNetAccept.cc b/src/iocore/net/UnixNetAccept.cc index b23f617f4fb..9046dbaf086 100644 --- a/src/iocore/net/UnixNetAccept.cc +++ b/src/iocore/net/UnixNetAccept.cc @@ -112,7 +112,7 @@ net_accept(NetAccept *na, void *ep, bool blockable) if (res == -EAGAIN || res == -ECONNABORTED || res == -EPIPE) { goto Ldone; } - if (na->action_->server.load(std::memory_order_acquire) != nullptr) { + if (na->action_->is_listening()) { if (!blockable) { na->action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); } else { @@ -387,7 +387,7 @@ NetAccept::do_blocking_accept(EThread *t) case -1: [[fallthrough]]; default: - if (action_->server.load(std::memory_order_acquire) != nullptr) { + if (action_->is_listening()) { SCOPED_MUTEX_LOCK(lock, action_->mutex ? action_->mutex : t->mutex, t); action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); Warning("accept thread received fatal error: errno = %d", errno); @@ -580,7 +580,7 @@ NetAccept::acceptFastEvent(int event, void *ep) check_transient_accept_error(res); goto Ldone; } - if (action_->server.load(std::memory_order_acquire) != nullptr) { + if (action_->is_listening()) { action_->continuation->handleEvent(EVENT_ERROR, reinterpret_cast(res)); } goto Lerror;