From efc899a1b03cb96ef02a54b37e7de941ae22230e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 23 Sep 2026 11:43:43 -0400 Subject: [PATCH] [MG_Remote, MG_Test, scripts] (P7): bound the pre-auth cap and pause accepting when out of descriptors --- MobileGL/MG_Remote/Server/PreAuthGate.h | 13 +++- MobileGL/MG_Remote/Server/ServerMain.cpp | 28 ++++++++- .../MG_Remote/Transport/SocketTransport.cpp | 8 ++- .../MG_Remote/Transport/SocketTransport.h | 4 ++ MobileGL/MG_Test/Wire/PreAuthGateTest.cpp | 10 +++ scripts/ci/tcp_supervisor_smoke.py | 63 +++++++++++++++++-- 6 files changed, 115 insertions(+), 11 deletions(-) diff --git a/MobileGL/MG_Remote/Server/PreAuthGate.h b/MobileGL/MG_Remote/Server/PreAuthGate.h index b7dce40e..d0ceeaaf 100644 --- a/MobileGL/MG_Remote/Server/PreAuthGate.h +++ b/MobileGL/MG_Remote/Server/PreAuthGate.h @@ -89,6 +89,11 @@ namespace MobileGL::MG_Remote::Server { // port probe; one that held its slot longer is a failure. A real client writes its first frame // the moment it connects, so it is never on the wrong side of this. inline constexpr std::uint32_t kPreAuthProbeGraceMs = 250; + // MOBILEGL_IPC_PREAUTH_MAX never exceeds this. Every pending connection is a descriptor the + // supervisor holds, and an unbounded knob would let a connection flood walk it into + // RLIMIT_NOFILE (the supervisor also survives that now - it pauses accepting - but a knob + // should not be the way in). + inline constexpr std::uint32_t kPreAuthMaxPendingCeiling = 256; // The refusal details a peer (and the log) sees. Named once so the supervisor and the unit test // cannot drift on the words; the TCP lane's scripts mirror them rather than parse them, so a @@ -108,8 +113,9 @@ namespace MobileGL::MG_Remote::Server { // MOBILEGL_IPC_PREAUTH_MS: how long a new connection has to present a complete first // frame. A real client writes its Hello (or DataBind) the moment it connects. std::uint32_t deadlineMs = 2000; - // MOBILEGL_IPC_PREAUTH_MAX: connections the supervisor holds unauthenticated at once; one - // more is refused Busy at accept. + // MOBILEGL_IPC_PREAUTH_MAX: connections the supervisor holds unauthenticated at once, at + // most kPreAuthMaxPendingCeiling. When they are all taken a newcomer displaces the busiest + // address's oldest one, or is refused Busy at accept (ChoosePendingToDisplace). std::uint32_t maxPending = 8; // MOBILEGL_IPC_AUTH_BACKOFF_AFTER: failures from one address before it is refused at // accept. 0 turns the backoff off (the fuzz arm does, to send many malformed frames from @@ -124,7 +130,8 @@ namespace MobileGL::MG_Remote::Server { static PreAuthKnobs FromEnvironment() { PreAuthKnobs knobs; knobs.deadlineMs = Read("MOBILEGL_IPC_PREAUTH_MS", knobs.deadlineMs, false); - knobs.maxPending = Read("MOBILEGL_IPC_PREAUTH_MAX", knobs.maxPending, false); + knobs.maxPending = + std::min(Read("MOBILEGL_IPC_PREAUTH_MAX", knobs.maxPending, false), kPreAuthMaxPendingCeiling); knobs.backoffAfter = Read("MOBILEGL_IPC_AUTH_BACKOFF_AFTER", knobs.backoffAfter, true); knobs.backoffBaseMs = Read("MOBILEGL_IPC_AUTH_BACKOFF_MS", knobs.backoffBaseMs, false); return knobs; diff --git a/MobileGL/MG_Remote/Server/ServerMain.cpp b/MobileGL/MG_Remote/Server/ServerMain.cpp index 02fa7d81..21b0dc44 100644 --- a/MobileGL/MG_Remote/Server/ServerMain.cpp +++ b/MobileGL/MG_Remote/Server/ServerMain.cpp @@ -714,16 +714,23 @@ public: std::vector fds; for (;;) { Reap(); - fds.clear(); - fds.push_back({m_listener, POLLIN, 0}); - for (const auto& peer : m_pending) fds.push_back({peer.fd, POLLIN, 0}); auto now = Clock::now(); + // While accepting is paused (out of descriptors, AcceptNew) the listener sits out the + // poll - it stays readable, and would spin the loop - and the pause's end is a wakeup. + const bool acceptPaused = now < m_acceptResumeAt; + fds.clear(); + fds.push_back({acceptPaused ? -1 : m_listener, POLLIN, 0}); + for (const auto& peer : m_pending) fds.push_back({peer.fd, POLLIN, 0}); // 250 ms is the old accept timeout, i.e. how often a finished child is reaped while // nothing arrives; a pending deadline that falls sooner shortens it. std::int64_t timeout = 250; for (const auto& peer : m_pending) timeout = std::clamp( std::chrono::duration_cast(peer.deadline - now).count(), 0, timeout); + if (acceptPaused) + timeout = std::clamp( + std::chrono::duration_cast(m_acceptResumeAt - now).count() + 1, 0, + timeout); const int polled = ::poll(fds.data(), static_cast(fds.size()), static_cast(timeout)); if (polled < 0 && errno != EINTR) { WireLogError("MG_Remote server: supervisor poll failed: %s", std::strerror(errno)); @@ -823,6 +830,18 @@ private: int fd = -1; const auto accepted = SocketTransport::AcceptOne(m_listener, 0, &fd); if (accepted == MOBILEGL_ERR_TIMEOUT) return 0; + if (accepted == MOBILEGL_ERR_OUT_OF_MEMORY) { + // f2-auth fix round. Out of descriptors (or memory) is not a dead listener: the + // connection waits in the backlog, and pending connections give descriptors back + // as they are judged or reach their deadline. The first cut returned 73 here, so + // a connection flood against a low RLIMIT_NOFILE (or a large pending cap) took the + // supervisor down. Now accepting pauses for kAcceptPauseMs and the loop goes on. + m_acceptResumeAt = now + std::chrono::milliseconds(kAcceptPauseMs); + WireLogError("MG_Remote server: accept paused for %u ms: out of descriptors or memory with %zu " + "connections awaiting authentication", + kAcceptPauseMs, m_pending.size()); + return 0; + } if (accepted != MOBILEGL_OK) return 73; std::string address = PeerAddress(fd); if (const std::uint32_t retry = m_backoff.RetryAfterMs(address, now); retry != 0) { @@ -979,6 +998,9 @@ private: int m_handoff = -1; std::vector m_pending; std::vector m_ready; + // Accepting resumes at this time after accept(2) ran out of descriptors or memory. + static constexpr std::uint32_t kAcceptPauseMs = 100; + Clock::time_point m_acceptResumeAt{}; }; } diff --git a/MobileGL/MG_Remote/Transport/SocketTransport.cpp b/MobileGL/MG_Remote/Transport/SocketTransport.cpp index 86d0768e..937722a9 100644 --- a/MobileGL/MG_Remote/Transport/SocketTransport.cpp +++ b/MobileGL/MG_Remote/Transport/SocketTransport.cpp @@ -486,7 +486,13 @@ namespace MobileGL::MG_Remote::Transport { if (fd < 0) { // The same two non-failures AcceptPair re-polls on. if (errno == EINTR || errno == ECONNABORTED) continue; - WireLogError("MG_Remote SocketTransport: accept failed: %s", std::strerror(errno)); + const int error = errno; + WireLogError("MG_Remote SocketTransport: accept failed: %s", std::strerror(error)); + // Out of descriptors or memory is the PROCESS's state, not the listener's: the + // connection waits in the backlog (f2-auth fix round; the TCP supervisor pauses + // and retries instead of exiting 73). + if (error == EMFILE || error == ENFILE || error == ENOBUFS || error == ENOMEM) + return MOBILEGL_ERR_OUT_OF_MEMORY; return MOBILEGL_ERR_TRANSPORT_CLOSED; } ::fcntl(fd, F_SETFD, FD_CLOEXEC); diff --git a/MobileGL/MG_Remote/Transport/SocketTransport.h b/MobileGL/MG_Remote/Transport/SocketTransport.h index 440716c1..f7d42d1b 100644 --- a/MobileGL/MG_Remote/Transport/SocketTransport.h +++ b/MobileGL/MG_Remote/Transport/SocketTransport.h @@ -123,6 +123,10 @@ namespace MobileGL::MG_Remote::Transport { static MobileGLResult ConnectDataConnection(const std::string& path, std::uint32_t timeoutMs, MobileGLByteSpan firstFrame, int* outFd); // Accepts ONE connection (TCP options applied, CLOEXEC). MOBILEGL_ERR_TIMEOUT when none. + // MOBILEGL_ERR_OUT_OF_MEMORY when accept(2) ran out of descriptors or memory (EMFILE, + // ENFILE, ENOBUFS, ENOMEM): the connection stays in the backlog and the listener is fine, + // so a caller that serves many peers can wait and try again (the TCP supervisor does); + // TRANSPORT_CLOSED for every other failure. static MobileGLResult AcceptOne(int listenFd, std::uint32_t timeoutMs, int* outFd); // Reads EXACTLY one framed message from a raw socket - the header, then the payload, // and not one byte more. A DataBind is followed on the same connection by StreamLink's diff --git a/MobileGL/MG_Test/Wire/PreAuthGateTest.cpp b/MobileGL/MG_Test/Wire/PreAuthGateTest.cpp index ddfc5e0d..fb6a57a7 100644 --- a/MobileGL/MG_Test/Wire/PreAuthGateTest.cpp +++ b/MobileGL/MG_Test/Wire/PreAuthGateTest.cpp @@ -303,4 +303,14 @@ TEST(PreAuthGateTest, TheKnobsReadTheirEnvironmentAndKeepTheirDefaultsOtherwise) EXPECT_EQ(knobs.backoffAfter, 5u); EXPECT_EQ(knobs.backoffBaseMs, 1000u); } + // f2-auth fix round: the pending cap is bounded, whatever it is set to - every pending + // connection is a descriptor. Red once by dropping the clamp: 100000 comes back. + { + ScopedEnvironment b("MOBILEGL_IPC_PREAUTH_MAX", "100000"); + EXPECT_EQ(Server::PreAuthKnobs::FromEnvironment().maxPending, Server::kPreAuthMaxPendingCeiling); + } + { + ScopedEnvironment b("MOBILEGL_IPC_PREAUTH_MAX", "256"); + EXPECT_EQ(Server::PreAuthKnobs::FromEnvironment().maxPending, 256u); + } } diff --git a/scripts/ci/tcp_supervisor_smoke.py b/scripts/ci/tcp_supervisor_smoke.py index 6b272013..ff6a9d9b 100644 --- a/scripts/ci/tcp_supervisor_smoke.py +++ b/scripts/ci/tcp_supervisor_smoke.py @@ -22,7 +22,9 @@ refusal (`data_bind_queued_before_bound`). The f2-auth fix round added that the shared between addresses (`pending_queue_is_shared`) and that an address holding slots with silent connections is backed off (`silent_slot_holder_backed_off`), that routing DataBinds does not stall the supervisor (`data_bind_routing_does_not_stall`), and that a child which refuses its Hello -refuses the DataBinds queued to it (`data_bind_queued_before_refused_hello`, ID-P7-44). The five +refuses the DataBinds queued to it (`data_bind_queued_before_refused_hello`, ID-P7-44), and that +running out of descriptors pauses accepting instead of ending the supervisor +(`accept_out_of_descriptors`). The five malformed-frame shapes are fuzz arm 1's own entry, scripts/ci/ph_fuzz_control_frames.py. It needs flatc, which is deliberately absent from the default build graph (gen_protocol.py's @@ -38,6 +40,7 @@ import json import os from pathlib import Path import re +import resource import shutil import signal import socket @@ -234,18 +237,25 @@ def refused_listen(server, log, endpoint, token): @contextmanager -def supervisor(server, log, same_build=False, serve=True, extra_env=None): +def supervisor(server, log, same_build=False, serve=True, extra_env=None, nofile=None): """A supervisor on its own free loopback port. `serve=False` is the single-session shape (no fork; the process IS the session and exits with it), which `single_session_listener` below needs and nothing else does. `extra_env` sets supervisor knobs (PH-7 (5)'s pre-auth - deadline, pending cap and backoff) for the controls that need non-default ones.""" + deadline, pending cap and backoff) for the controls that need non-default ones. `nofile` + lowers the supervisor's soft RLIMIT_NOFILE, for the descriptor-exhaustion control.""" port = free_loopback_port() base = log.with_suffix('.library.log') env = supervisor_environment(base, kToken, same_build=same_build) env.update(extra_env or {}) + + def limit(): + if nofile is not None: + resource.setrlimit(resource.RLIMIT_NOFILE, (nofile, resource.getrlimit(resource.RLIMIT_NOFILE)[1])) + with log.open('wb') as output: child = subprocess.Popen([server, f'tcp://127.0.0.1:{port}'] + (['--serve'] if serve else []), - env=env, stdout=output, stderr=output, start_new_session=True) + env=env, stdout=output, stderr=output, start_new_session=True, + preexec_fn=limit if nofile is not None else None) try: # Log.cpp's RoleLogPath: the base name is a BASE, and the server role writes # `.server`. Yielded rather than recomputed at the call site, so the one @@ -972,6 +982,51 @@ def main(): l.strip() for l in named.splitlines() if reason in l)} assert process.poll() is None, 'the pre-auth limits killed the supervisor' + # ---- f2-auth fix round: OUT OF DESCRIPTORS, THE SUPERVISOR PAUSES ACCEPTING, NOT EXITS ---- + # + # accept(2) failing with EMFILE used to be "the listener failed": the supervisor logged + # its summary and exited 73, so a connection flood against a low RLIMIT_NOFILE (or a large + # MOBILEGL_IPC_PREAUTH_MAX - now clamped to 256) took the server down for everybody. Here + # the supervisor runs with a soft limit of 16 descriptors (five in use when idle) and a + # pending cap it cannot reach (64), and 48 silent connections are opened (the listen + # backlog is 8, so they arrive in waves - the first waves alone pass the limit): it must + # name the pause, stay up, and - once the flood is gone - still answer a Hello + # (fingerprint 0, so the answer is the child's Refuse{WireFingerprint}). The backoff is + # off: the flood's failures are not this control's subject. RED before the fix: the + # supervisor has exited. + exhaust = {'MOBILEGL_IPC_PREAUTH_MAX': '64', 'MOBILEGL_IPC_PREAUTH_MS': '3000', + 'MOBILEGL_IPC_AUTH_BACKOFF_AFTER': '0'} + with supervisor(args.server, args.out / 'descriptors.log', extra_env=exhaust, nofile=16) as ( + port, process, serverLog): + flood = [] + try: + # The first connect waits for the supervisor to be listening (connect_control's + # retry); the rest are non-blocking: a blocking one stalls this loop for its whole + # timeout once the backlog is full, and the flood would never outrun the deadlines. + flood.append(connect_control(port)) + for _ in range(47): + peer = socket.socket() + flood.append(peer) + peer.setblocking(False) + try: + peer.connect(('127.0.0.1', port)) + except (BlockingIOError, ConnectionRefusedError): + pass # refused is the red shape: the supervisor is gone + paused = wait_for_log(serverLog, 'accept paused for', seconds=5) + alive = process.poll() is None + finally: + for peer in flood: + peer.close() + assert alive, f'the supervisor exited ({process.poll()}) when accept ran out of descriptors; see {serverLog}' + assert paused is not None, f'the supervisor never named its accept pause in {serverLog}' + time.sleep(1.0) # every flood connection has been read as closed (or is still in the backlog) + after_flood = exchange(port, schema, hello(schema, flatbuffers)) + assert after_flood.get('code') == 2 and after_flood.get('expected', 0), ( + f'after the descriptor flood a Hello was answered {after_flood}') + assert process.poll() is None, 'the supervisor died after the descriptor flood' + evidence['accept_out_of_descriptors'] = {'line': next( + l.strip() for l in paused.splitlines() if 'accept paused for' in l), 'afterFlood': after_flood} + # ---- ID-P7-44, f2-auth fix round: A CHILD THAT REFUSES ITS HELLO REFUSES WHAT WAS QUEUED -- # # The supervisor forwards DataBinds to a child from the moment it forks it. A child that