From 49fecc168641b0d4c47d1d74e457d65b9a4b4c2c Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 10:17:29 -0400 Subject: [PATCH] [Fix] (DirectGLES/Managers, MG_Backend/Init, MG_Remote/Server): N-7 m-5's freed-base null is transport-guarded like the other two split hunks so monolith transport in a split build changes nothing by construction; N-8 the second consumer-mask check compares MGPipeGetResourceOps() against the table step 2 saw and refuses a replaced table as Fatal{ConsumerMaskLie, resource ops table replaced} (the re-run alone could not see a client-registered table); ServerVerbSink::ReadbackScratchBytes for ID-49's tight-size gate --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 10 +++++- MobileGL/MG_Backend/Init.cpp | 34 +++++++++++++++++---- MobileGL/MG_Remote/Server/PipeApplier.h | 4 +++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 37f937a1..a9eda1a4 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2980,7 +2980,15 @@ namespace MobileGL::MG_Backend::DirectGLES { // is still live; nulling it then would drop the reduced path's own bytes (it did, // and TriangleScenario read the wrong VBO). HasShadow is the discriminator: false // after DropAll, true after an ordinary Adopt. - if (!ServerStaged().HasShadow(resource)) { + // + // TRANSPORT-GUARDED like the other two split hunks in this file (review v2 N-7): + // under MONOLITH transport in a disaggregated build the store never copies, Adopt + // never sets m_any, HasShadow answers false for everything, and this null would + // run on every twin's first ensure - harmless there only because liveHostBase() + // prefers MappedData() under monolith, and "under monolith nothing changes" should + // be true by construction rather than by luck. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith && + !ServerStaged().HasShadow(resource)) { resource->hostBytes = nullptr; } #endif diff --git a/MobileGL/MG_Backend/Init.cpp b/MobileGL/MG_Backend/Init.cpp index 451b9423..a7dd7a35 100644 --- a/MobileGL/MG_Backend/Init.cpp +++ b/MobileGL/MG_Backend/Init.cpp @@ -118,6 +118,13 @@ namespace MobileGL::MG_Backend { } } + // The resource op table the SERVER's backend registered at step 1 (BackendObject_DirectGLES:: + // Initialize -> RegisterBufferBackendOps), as step 2 saw it. Step 5 compares against it + // (review v2 N-8): a client object that registered a table of its own would have made + // AssertConsumerMaskIsHonest's "is a table registered" answer TRUE, so re-asking that + // question could never notice the swap - only the pointer can. + const MG_Pipe::MGPipeResourceOps* g_resourceOpsAtStep2 = nullptr; + // The single hook (ARCHITECTURE.md:29). Returns false when the split could not be // brought up, and the caller then REFUSES TO CONTINUE rather than falling back to the // switch below - a fallback here is "the split lane ran monolith and went green". @@ -136,6 +143,7 @@ namespace MobileGL::MG_Backend { Server::ServerSession& session = Server::ServerSessionInstance(); const Uint64 consumed = ConsumedSubsystemsFor(MG_Config::ActiveBackendType); AssertConsumerMaskIsHonest(consumed); + g_resourceOpsAtStep2 = MG_Pipe::MGPipeGetResourceOps(); session.SetConsumedSubsystems(consumed); // ZERO IS THE EXPLICIT ANSWER FOR P5, not an omission (ServerSession.h's block): // every optional capability bit belongs to the package that owns its question, and @@ -215,13 +223,27 @@ namespace MobileGL::MG_Backend { MGLOG_W("Failed to initialize MobileGL backend libraries for the remote object"); return; } - // m-6: the honesty cross-check runs a SECOND time, now that step 4's - // pActiveBackendObject (the client's BackendObject_Remote) exists and its - // Initialize() has run inside InitSpecificBackendLibs. Anything the client object - // registered into MGPipeSetResourceOps after the step-2 check is invisible to that - // first call; re-asserting here costs one call and closes the window in which a - // client-registered g_resourceOps would flip the answer under the applier's feet. + // m-6, re-worded per review v2 N-8. The honesty cross-check runs a SECOND time, now + // that step 4's pActiveBackendObject (the client's BackendObject_Remote) exists and + // its Initialize() has run inside InitSpecificBackendLibs. What the re-run CAN catch + // is a table that was REMOVED between step 2 and here (the claim would then be a lie + // again). What it cannot catch - and its first comment claimed it could - is a client + // object that REGISTERED a table of its own: that leaves "is a table registered" + // true. Only the pointer tells those apart, so the table is compared against the one + // step 2 saw and a swap is refused by name: the applier would otherwise dispatch the + // server's resource records into the CLIENT object's table under its feet. AssertConsumerMaskIsHonest(ConsumedSubsystemsFor(MG_Config::ActiveBackendType)); + if (MG_Pipe::MGPipeGetResourceOps() != g_resourceOpsAtStep2) { + MGLOG_F("MGPipe: Fatal{ConsumerMaskLie, \"resource ops table replaced\"} - the " + "resource op table MGPipeGetResourceOps() answers with is not the one the " + "server's backend registered at step 1 (%p now, %p then). Something between " + "ServerSession::Accept and the client object's Initialize() registered its " + "own table, and the applier would dispatch every resource record into it. A " + "mask is a statement about the server's backend, and so is the table", + static_cast(MG_Pipe::MGPipeGetResourceOps()), + static_cast(g_resourceOpsAtStep2)); + std::abort(); + } LogBackendInfo(); return; } diff --git a/MobileGL/MG_Remote/Server/PipeApplier.h b/MobileGL/MG_Remote/Server/PipeApplier.h index c11ca7e5..e979578b 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.h +++ b/MobileGL/MG_Remote/Server/PipeApplier.h @@ -128,6 +128,10 @@ namespace MobileGL::MG_Remote::Server { Uint64 Presents() const { return m_presents; } Uint64 LastPresentSerial() const { return m_lastPresentSerial; } Uint64 ReadbackBytes() const { return m_readbackBytes; } + // ID-49's tight-size control reads this: the scratch a read_pixels grew to. It must equal + // the tight w*h*bpp extent of the read, never the client's DstSize - a scratch sized from + // DstSize is exactly the heap overflow codex 1 found, one field over. + Uint64 ReadbackScratchBytes() const { return static_cast(m_readbackScratch.size()); } private: const MG_Backend::GlobalBackendFunctionsTable* Table(const char* verb) const;