From 0b566b4869b9e86c90162befd87f9882b6dd143c Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 07:09:45 +0000 Subject: net: a refused thread spawn drops one peer, not the node std::thread throws std::system_error when pthread_create is refused (RLIMIT_NPROC, a container's pids cgroup, memory pressure), and nothing caught it, so the node died by std::terminate. Threads are spawned per peer connection, so the pressure that triggers this scales with peer count, which is attacker-influenced. Observed killing a healthy participant worker mid-mining. Every spawn site in the node build, and what a failure now does: - btftunnel.cpp: both tunnel pumps are guarded. The inbound pump starts first because it owns cleanup of the pump end and rendezvous socket; if the second spawn is refused, shutdown() wakes the first out of its read and it cleans up itself -- closing those descriptors from the failure path instead would race it onto reused descriptors. Both paths return INVALID_SOCKET, which every caller already treats as "this connection failed". - btfrv.cpp: the relay's accept loop drops the connection it cannot serve and stops accepting for a moment -- longer after consecutive failures, capped at 5s -- instead of dying at an arbitrary one. The per-pair second pump now runs on the handling thread itself, leaving exactly one spawn to guard and nothing running yet to race. - main.cpp: a participant that cannot spawn its hash workers stops the ones that did start, drops the pool connection, and returns to the reconnect loop, the same way an ordinary disconnect does. - randomx_pow.cpp: dataset init joins the workers it got, releases the dataset, and stays in light mode -- the same trade the allocation failure path above it already makes. - rpc.cpp, net.cpp: the remaining unchecked _beginthread calls (which report failure by value, they cannot throw) now say precisely what did not start. The pool's accept and payout threads failing silently left the pool running deaf -- nobody accepts, nobody gets paid, and nothing in the log says why. Left alone deliberately: the per-relay accept spawns (rpc.cpp) already check, log and retry on the next poll; startup-critical spawns in net.cpp keep failing hard; the selftest-only std::thread uses may abort a selftest run, which is acceptable where dying over one peer is not. Verified: full node rebuilds clean and links; pool-stratum selftest passes; a scratch harness under RLIMIT_NPROC=1 confirms _beginthread reports -1 and std::thread throws system_error, the premises of every path above. --- src/btftunnel.cpp | 52 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 4 deletions(-) (limited to 'src/btftunnel.cpp') diff --git a/src/btftunnel.cpp b/src/btftunnel.cpp index 63d178b..2b3f818 100644 --- a/src/btftunnel.cpp +++ b/src/btftunnel.cpp @@ -25,8 +25,10 @@ typedef int SOCKET; #endif #include "sockcount.h" #include +#include #include #include +#include namespace btf { @@ -121,18 +123,60 @@ static void PumpIn(SOCKET pumpEnd, RvSocket rv, std::vector key) // Start the two pumps and hand back the app-facing socket. When both pumps // finish, close the pump end and the rendezvous socket. +// +// Every tunnel is two threads, and a tunnel is one peer connection (a .btf +// dial or accept, a pool miner, a participant's pool session) -- so the +// thread count scales with peer count, which is attacker-influenced. When +// pthread_create is refused std::thread throws std::system_error; uncaught it +// reached std::terminate and took the whole process down over one connection +// it could not serve. Both spawns are guarded, and the failure paths return +// INVALID_SOCKET, which every caller already handles by dropping just that +// connection. static btf_socket_t StartTunnel(RvSocket rv, const unsigned char shared[32]) { btf_socket_t appEnd, pumpEnd; if (!MakeLocalPair(appEnd, pumpEnd)) { RvClose(rv); return INVALID_SOCKET; } std::vector key(shared, shared + 32); - std::thread(PumpOut, pumpEnd, rv, key).detach(); - std::thread([pumpEnd, rv, key]() { - PumpIn(pumpEnd, rv, key); + + // The inbound pump starts first because it owns the cleanup: it closes + // pumpEnd and rv when it exits. That ownership is what makes the second + // spawn's failure path below race-free. + try + { + std::thread([pumpEnd, rv, key]() { + PumpIn(pumpEnd, rv, key); + BtfCloseSocket(pumpEnd); + RvClose(rv); + }).detach(); + } + catch (const std::system_error& e) + { + // Nothing is running yet, so closing everything here is safe. + fprintf(stderr, "btf tunnel: cannot spawn inbound pump (%s), tunnel dropped\n", + e.what()); BtfCloseSocket(pumpEnd); + BtfCloseSocket(appEnd); RvClose(rv); - }).detach(); + return INVALID_SOCKET; + } + + try + { + std::thread(PumpOut, pumpEnd, rv, key).detach(); + } + catch (const std::system_error& e) + { + // The inbound pump is parked in RvReadN(rv). shutdown() wakes it, and + // it closes pumpEnd and rv itself -- closing them here instead would + // race exactly that, and a reused descriptor is how an unrelated + // socket gets closed. + fprintf(stderr, "btf tunnel: cannot spawn outbound pump (%s), tunnel dropped\n", + e.what()); + shutdown((SOCKET)rv, SD_BOTH); + BtfCloseSocket(appEnd); + return INVALID_SOCKET; + } return appEnd; } -- cgit v1.2.3 From 0a8c445742ce871c6b9352bf406d2232a6fd3134 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 17:39:50 +0000 Subject: tunnel: give both pumps shared ownership of the sockets they close PumpIn's wrapper closed pumpEnd and the rendezvous socket the moment it exited, without waiting for PumpOut, which then called shutdown() on both at its own exit. Which of the two got there first decided whether that shutdown addressed a live descriptor or a number the kernel had already handed back out -- in which case it tears down an unrelated connection. The inbound pump wins that race whenever the rendezvous socket dies before the app socket, which is the ordinary shape of a relay restart or a far side hanging up. The failure path added in 0b566b4 has the same hole. Its catch shuts down rv to wake an inbound pump it assumes is parked in RvReadN. PumpIn does not park when the rendezvous connection is already dead: it returns on its first length read and its wrapper closes both descriptors. A refused pthread_create and a dead rv are not independent -- both are symptoms of a loaded process -- so that is the likely case, not the exotic one. Both descriptors now belong to a shared_ptr the two pumps hold for the whole of their bodies, so the close happens after the last pump has stopped touching them and no shutdown can precede it. appEnd is unchanged: the caller gets it on success, StartTunnel closes it on every failure path, no pump ever touches it. 0b566b4's message says of that catch that "closing those descriptors from the failure path instead would race it". True as far as it goes, but it reads as though what the code did instead was safe, and it was not -- the shutdown raced the same close. There was no ordering there to prefer, only a missing owner. Measured against the real code with the relay in a separate process, every close() and shutdown() traced and pthread_create refused on demand: normal teardown, 25 tunnels 29 shutdowns on a closed fd -> 0 refused outbound spawn, 25 25 shutdowns on a closed fd -> 0 Close counts are identical before and after (8 and 5 per tunnel, every one accounted for) and live descriptors return to their starting count, so the defect was only ever the ordering, never a leak or a double close. --- src/btftunnel.cpp | 85 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 68 insertions(+), 17 deletions(-) (limited to 'src/btftunnel.cpp') diff --git a/src/btftunnel.cpp b/src/btftunnel.cpp index 2b3f818..4acca1a 100644 --- a/src/btftunnel.cpp +++ b/src/btftunnel.cpp @@ -26,6 +26,8 @@ typedef int SOCKET; #include "sockcount.h" #include #include +#include +#include #include #include #include @@ -121,9 +123,44 @@ static void PumpIn(SOCKET pumpEnd, RvSocket rv, std::vector key) shutdown((SOCKET)rv, SD_BOTH); } +// The pump end and the rendezvous socket, owned jointly by whoever is still +// using them. Closed once, by the last reference to go. +// +// Joint ownership rather than "the inbound pump owns the cleanup", because +// either pump can reach its exit first and BOTH touch BOTH descriptors on the +// way out -- PumpOut ends in shutdown(rv), shutdown(pumpEnd) and PumpIn ends in +// the mirror image. A single owner therefore has no exit at which the other is +// guaranteed to be finished. Holding a reference for the whole body of each +// pump gives it one: the close cannot happen until the last pump has stopped +// touching the descriptors, so no shutdown() can land on a number that has +// already been closed and handed back out by the kernel to an unrelated +// connection. +struct TunnelEnds +{ + SOCKET pumpEnd; + RvSocket rv; + TunnelEnds(SOCKET p, RvSocket r) : pumpEnd(p), rv(r) {} + ~TunnelEnds() { BtfCloseSocket(pumpEnd); RvClose(rv); } +private: + TunnelEnds(const TunnelEnds&); + TunnelEnds& operator=(const TunnelEnds&); +}; + // Start the two pumps and hand back the app-facing socket. When both pumps // finish, close the pump end and the rendezvous socket. // +// That sentence used to be a comment describing something the code did not do. +// The inbound pump closed both descriptors as soon as it exited, without +// waiting for the outbound pump, which then called shutdown() on both at its +// own exit. Whether that shutdown hit a live descriptor or a closed one came +// down to which pump got there first, and the rendezvous socket dying before +// the app socket -- a relay restart, the far side hanging up -- is precisely +// the order that puts the inbound pump ahead. Measured over 25 such teardowns +// against the real code: 29 shutdowns landed on an already-closed descriptor. +// +// appEnd is not part of this. It is handed to the caller on success and closed +// here on every failure path; no pump ever touches it. +// // Every tunnel is two threads, and a tunnel is one peer connection (a .btf // dial or accept, a pool miner, a participant's pool session) -- so the // thread count scales with peer count, which is attacker-influenced. When @@ -139,41 +176,55 @@ static btf_socket_t StartTunnel(RvSocket rv, const unsigned char shared[32]) std::vector key(shared, shared + 32); - // The inbound pump starts first because it owns the cleanup: it closes - // pumpEnd and rv when it exits. That ownership is what makes the second - // spawn's failure path below race-free. + // make_shared, not shared_ptr(new ...): it allocates before it constructs, + // so it either comes back owning both descriptors or throws having taken + // ownership of neither. The two-step form can run the deleter and then + // throw, which would close both here and again below. + std::shared_ptr ends; + try + { + ends = std::make_shared(pumpEnd, rv); + } + catch (const std::bad_alloc&) + { + fprintf(stderr, "btf tunnel: out of memory starting tunnel, tunnel dropped\n"); + BtfCloseSocket(pumpEnd); + BtfCloseSocket(appEnd); + RvClose(rv); + return INVALID_SOCKET; + } + try { - std::thread([pumpEnd, rv, key]() { - PumpIn(pumpEnd, rv, key); - BtfCloseSocket(pumpEnd); - RvClose(rv); - }).detach(); + std::thread([ends, key]() { PumpIn(ends->pumpEnd, ends->rv, key); }).detach(); } catch (const std::system_error& e) { - // Nothing is running yet, so closing everything here is safe. + // Nothing is running yet: `ends` is the only reference, and returning + // drops it, which is what closes pumpEnd and rv. fprintf(stderr, "btf tunnel: cannot spawn inbound pump (%s), tunnel dropped\n", e.what()); - BtfCloseSocket(pumpEnd); BtfCloseSocket(appEnd); - RvClose(rv); return INVALID_SOCKET; } try { - std::thread(PumpOut, pumpEnd, rv, key).detach(); + std::thread([ends, key]() { PumpOut(ends->pumpEnd, ends->rv, key); }).detach(); } catch (const std::system_error& e) { - // The inbound pump is parked in RvReadN(rv). shutdown() wakes it, and - // it closes pumpEnd and rv itself -- closing them here instead would - // race exactly that, and a reused descriptor is how an unrelated - // socket gets closed. + // The inbound pump may be parked in RvReadN(rv) or may already have + // exited -- when the spawn is refused because the process is loaded, + // the rendezvous socket is often dead for the same reason, and then + // PumpIn returns on its first length read. Either way this thread + // still holds a reference, so both descriptors are still open: the + // shutdown wakes the pump if it is parked and is harmless if it is + // not. The close belongs to whichever reference goes last. fprintf(stderr, "btf tunnel: cannot spawn outbound pump (%s), tunnel dropped\n", e.what()); - shutdown((SOCKET)rv, SD_BOTH); + shutdown((SOCKET)ends->rv, SD_BOTH); + shutdown(ends->pumpEnd, SD_BOTH); BtfCloseSocket(appEnd); return INVALID_SOCKET; } -- cgit v1.2.3