[Fix] (Remote): offer inproc no shared images and refuse them on its server

MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp

@@ -1659,7 +1659,10 @@ namespace MobileGL::MG_Impl::EGLImpl {
 #if MOBILEGL_BUILD_DISAGGREGATED
         // A split client's backend is the remote one, which forwards the shared-image verbs to
         // the server; the server's own apply thread and a monolith have no allocator behind them.
-        return StreamGateActive() && MG_Backend::pActiveBackendObject != nullptr;
+        // Nor has inproc: shared images carry memory between processes, and an inproc server is
+        // this process (its server refuses them too - ServerVerbSink::OnSharedImage).
+        return StreamGateActive() && MG_Config::Transport != MG_Config::TransportMode::InProcess &&
+               MG_Backend::pActiveBackendObject != nullptr;
 #else
         return false;
 #endif

MobileGL/MG_Remote/Client/ClientSession.cpp

@@ -3012,8 +3012,7 @@ namespace MobileGL::MG_Remote::Client {
     Bool ClientSession::AdoptT0() {
         if (!m_adoptT0Asked) return false;
         if (m_adoptT0Decision >= 0) return m_adoptT0Decision == 1;
-        const Bool peerIsThisProcess = m_transport != nullptr &&
-            Transport::AdoptT0::PeerIsThisProcess(m_transport->Role() == Transport::TransportRole::InProcess);
+        const Bool peerIsThisProcess = Transport::PeerIsThisProcess(m_transport);
         if (Caps().HasCap(MG_Pipe::kCapAdoptT0) && peerIsThisProcess &&
             !Caps().HasCap(MG_Pipe::kCapAdoptInProcess)) {
             // A server in this process offered the AHardwareBuffer arm. The stores are this

MobileGL/MG_Remote/Server/PipeApplier.cpp

@@ -390,6 +390,9 @@ namespace MobileGL::MG_Remote::Server {
         ServerSession* session = ServerSession::Active();
         if (session == nullptr) return false;
         SI::SessionHolder& holder = session->SharedImageHolder();
+        // Shared images carry memory between processes. A client in this process has none to
+        // carry, so it gets none - no platform buffer is ever allocated or imported for it.
+        const Bool peerIsThisProcess = Transport::PeerIsThisProcess(session->ControlTransport());
         const auto fill = [&reply](const SI::Image& image) {
             reply.ImageId = image.Id;
             reply.Modifier = image.Modifier;
@@ -403,6 +406,10 @@ namespace MobileGL::MG_Remote::Server {
         };
         switch (op.Op) {
         case MG_Pipe::kMGPSharedImageAllocate: {
+            if (peerIsThisProcess) {
+                MGLOG_W_ONCE("MG_Remote server: a shared-image allocation was refused: the client is this process");
+                return false;
+            }
             std::string why;
             SI::ImageRef image = SI::Allocate(op.Width, op.Height, op.Format, why);
             if (image == nullptr) {
@@ -436,6 +443,13 @@ namespace MobileGL::MG_Remote::Server {
                         static_cast<unsigned long long>(seq), why.c_str());
                 return false;
             }
+            if (peerIsThisProcess) {
+#if !defined(_WIN32)
+                ::close(fd);
+#endif
+                MGLOG_W_ONCE("MG_Remote server: a dma-buf import was refused: the client is this process");
+                return false;
+            }
             SI::ImageRef image = SI::Identify(fd, why);
             // YUV: the planes and colour hints ride in Damage (MGPImportPlaneWord). A buffer this
             // server did not allocate is taken too, through the labelled CPU-copy fallback

MobileGL/MG_Remote/Server/ServerSession.cpp

@@ -1160,8 +1160,7 @@ namespace MobileGL::MG_Remote::Server {
         // stores' own mappings, donated as monolith donates them, and no AHardwareBuffer, import
         // or POST is involved. A backend that cannot donate leaves the session on T2 - an
         // in-process session never takes the AHardwareBuffer arm below.
-        const Bool inProcess = m_transport != nullptr &&
-            Transport::AdoptT0::PeerIsThisProcess(m_transport->Role() == Transport::TransportRole::InProcess);
+        const Bool inProcess = Transport::PeerIsThisProcess(m_transport);
         if (inProcess) {
             const MG_Pipe::MGPipeResourceOps* ops = MG_Pipe::MGPipeGetResourceOps();
             if (MG_Config::Ipc.AllowAdoptT0 == 0) {

MobileGL/MG_Remote/Transport/AdoptT0.cpp

@@ -184,18 +184,6 @@ namespace MobileGL::MG_Remote::Transport::AdoptT0 {
         return test != nullptr ? test->HasAhb() : RealHasAhb();
     }
 
-    namespace {
-        std::atomic<bool> g_peerInAnotherProcessForTest{false};
-    } // namespace
-
-    bool PeerIsThisProcess(bool inProcessTransport) {
-        return inProcessTransport && !g_peerInAnotherProcessForTest.load(std::memory_order_acquire);
-    }
-
-    void SetPeerInAnotherProcessForTest(bool asIfCrossProcess) {
-        g_peerInAnotherProcessForTest.store(asIfCrossProcess, std::memory_order_release);
-    }
-
     bool AllocateHeld(std::uint64_t size, const void* seed, HeldStore& out, std::string& why) {
         const PlatformForTest* test = g_testPlatform.load(std::memory_order_acquire);
         return test != nullptr ? test->AllocateHeld(size, seed, out, why) : RealAllocateHeld(size, seed, out, why);

MobileGL/MG_Remote/Transport/AdoptT0.h

@@ -56,13 +56,9 @@ namespace MobileGL::MG_Remote::Transport::AdoptT0 {
     // True where AHardwareBuffer exists (Android). Everywhere else T0 is never granted.
     bool PlatformHasAhb();
 
-    // True when a session's peer is this very process (an in-process transport). Such a session
-    // never takes this header's arm: its stores are this process's memory already, so T0 there is
-    // the backend handing over its own store (kCapAdoptInProcess) or nothing (T2).
-    bool PeerIsThisProcess(bool inProcessTransport);
-    // Test seam: the protocol tests run over an in-process transport and stand for a peer in
-    // another process while this is set.
-    void SetPeerInAnotherProcessForTest(bool asIfCrossProcess);
+    // A session whose peer is this very process (Transport::PeerIsThisProcess) never takes this
+    // header's arm: its stores are this process's memory already, so T0 there is the backend
+    // handing over its own store (kCapAdoptInProcess) or nothing (T2).
 
     // ---- client half ----------------------------------------------------------------------
 

MobileGL/MG_Remote/Transport/ITransport.h

@@ -127,4 +127,12 @@ namespace MobileGL::MG_Remote::Transport {
         ITransport() = default;
     };
 
+    // True when the session on `transport` has this very process as its peer (an in-process
+    // transport). Its memory is the peer's already, so what exists only to carry memory between
+    // processes - T0's handle arm, shared images - is never offered on it.
+    bool PeerIsThisProcess(const ITransport* transport);
+    // Test seam: the protocol tests run over an in-process transport and stand for a peer in
+    // another process while this is set.
+    void SetPeerInAnotherProcessForTest(bool asIfCrossProcess);
+
 } // namespace MobileGL::MG_Remote::Transport

MobileGL/MG_Remote/Transport/InProcessTransport.cpp

@@ -13,6 +13,7 @@
 
 #include <MG_Util/Debug/Log.h>
 
+#include <atomic>
 #include <cerrno>
 #include <chrono>
 #include <condition_variable>
@@ -290,4 +291,17 @@ namespace MobileGL::MG_Remote::Transport {
 
     Doorbell& InProcessTransport::SelfDoorbell() { return m_channel->Bell(m_endpoint); }
 
+    namespace {
+        std::atomic<bool> g_peerInAnotherProcessForTest{false};
+    } // namespace
+
+    bool PeerIsThisProcess(const ITransport* transport) {
+        return transport != nullptr && transport->Role() == TransportRole::InProcess &&
+               !g_peerInAnotherProcessForTest.load(std::memory_order_acquire);
+    }
+
+    void SetPeerInAnotherProcessForTest(bool asIfCrossProcess) {
+        g_peerInAnotherProcessForTest.store(asIfCrossProcess, std::memory_order_release);
+    }
+
 } // namespace MobileGL::MG_Remote::Transport

MobileGL/MG_Test/Wire/GlamorEglSequenceTest.cpp

@@ -32,6 +32,9 @@
 #include <MG_Backend/BackendObjects.h>
 #include <MG_Impl/EGLImpl/EGLImpl.h>
 
+#include <fstream>
+#include <iterator>
+
 using namespace MobileGL;
 using namespace P12;
 
@@ -332,6 +335,52 @@ namespace {
         (void)EGL::Terminate(dpy);
     }
 
+    struct InprocReport {
+        Int32 initialized = 0;
+        Int32 current = 0;
+        Int32 dmabufImport = 0;
+        Int32 nativeFence = 0;
+        Int32 externalImage = 0;
+        Int32 allocated = 0;
+        Int32 eglError = 0;
+        char note[128] = {};
+    };
+
+    // An inproc client: its server is this process, so nothing that carries memory between
+    // processes is offered, asked for or allocated.
+    void RunInproc(InprocReport& r) {
+        static int fakeGbmDevice[16] = {};
+        const EGLDisplay dpy = EGL::GetPlatformDisplay(kPlatformGbm, fakeGbmDevice, nullptr);
+        EGLint major = 0, minor = 0;
+        r.initialized = dpy != EGL_NO_DISPLAY && EGL::Initialize(dpy, &major, &minor) == EGL_TRUE ? 1 : 0;
+        if (!r.initialized) return;
+        const char* exts = EGL::QueryString(dpy, EGL_EXTENSIONS);
+        r.dmabufImport = HasToken(exts, "EGL_EXT_image_dma_buf_import") ? 1 : 0;
+        r.nativeFence = HasToken(exts, "EGL_ANDROID_native_fence_sync") ? 1 : 0;
+        (void)EGL::BindAPI(EGL_OPENGL_API);
+        const EGLint core33[] = {EGL_CONTEXT_OPENGL_PROFILE_MASK, EGL_CONTEXT_OPENGL_CORE_PROFILE_BIT,
+                                 EGL_CONTEXT_MAJOR_VERSION, 3, EGL_CONTEXT_MINOR_VERSION, 3, EGL_NONE};
+        const EGLContext ctx = EGL::CreateContext(dpy, EGL_NO_CONFIG_KHR, EGL_NO_CONTEXT, core33);
+        r.current = ctx != EGL_NO_CONTEXT && EGL::MakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, ctx) == EGL_TRUE;
+        if (!r.current) {
+            r.eglError = EGL::GetError();
+            return;
+        }
+        // The GL extension list is where a split client asks the server whether it holds YUV images.
+        r.externalImage = HasGlExtension("GL_OES_EGL_image_external") ? 1 : 0;
+        // Asked anyway (a C-ABI or GLX caller does not look at the extensions first): refused.
+        auto* backend = MG_Backend::pActiveBackendObject.get();
+        MG_Backend::SharedImageExport image;
+        if (backend != nullptr && backend->AllocateSharedImage(64, 32, kXrgb8888, &image)) {
+            r.allocated = 1;
+            if (image.Fd >= 0) ::close(image.Fd);
+            (void)backend->ReleaseSharedImage(image.Id);
+        }
+        (void)EGL::MakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
+        (void)EGL::DestroyContext(dpy, ctx);
+        (void)EGL::Terminate(dpy);
+    }
+
     // The supervisor on an abstract unix socket, as the device's is: shared images travel as
     // descriptors on its aux socket, which a TCP endpoint does not have.
     bool LaunchUnixSupervisor(ServerProcess* server) {
@@ -350,8 +399,9 @@ namespace {
     }
 
     // One forked client running `run` against the server, its report back through a pipe.
+    // `server == nullptr` runs the client inproc, its server in its own process.
     template <class Report>
-    Report RunPeer(const ServerProcess& server, const std::string& clientLogBase, void (*run)(Report&)) {
+    Report RunPeer(const ServerProcess* server, const std::string& clientLogBase, void (*run)(Report&)) {
         Report report{};
         int fds[2] = {-1, -1};
         if (::pipe(fds) != 0) return report;
@@ -360,8 +410,8 @@ namespace {
         if (pid == 0) {
             ::close(fds[0]);
             ::setenv("MOBILEGL_LOG_FILE_PATH", clientLogBase.c_str(), 1);
-            ::setenv("MOBILEGL_TRANSPORT", "spawn", 1);
-            ::setenv("MOBILEGL_IPC_CONTROL", server.endpoint.c_str(), 1);
+            ::setenv("MOBILEGL_TRANSPORT", server != nullptr ? "spawn" : "inproc", 1);
+            if (server != nullptr) ::setenv("MOBILEGL_IPC_CONTROL", server->endpoint.c_str(), 1);
             ::setenv("MOBILEGL_BACKEND_TYPE", "DirectGLES", 1);
             ::setenv("MOBILEGL_IPC_SURFACE", "offscreen", 1);
             Report r{};
@@ -398,7 +448,7 @@ TEST(GlamorEglSequence, XwaylandFindsEverythingGlamorNeeds) {
     ASSERT_TRUE(LaunchUnixSupervisor(&server)) << server.Log();
     const std::string clientLog = "/tmp/mgl-glamor-client-" + std::to_string(::getpid()) + ".log";
     Debug::TruncateRoleLogs(clientLog.c_str());
-    const GlamorReport r = RunPeer<GlamorReport>(server, clientLog, RunGlamor);
+    const GlamorReport r = RunPeer<GlamorReport>(&server, clientLog, RunGlamor);
     ASSERT_EQ(r.display, 1) << r.note;
     EXPECT_EQ(r.platformGbmExtension, 1);
     ASSERT_EQ(r.initialized, 1);
@@ -430,7 +480,7 @@ TEST(GlamorEglSequence, ChromesGpuProcessGetsANativeFenceOfItsDmaBufWrites) {
     ASSERT_TRUE(LaunchUnixSupervisor(&server)) << server.Log();
     const std::string clientLog = "/tmp/mgl-chrome-fence-client-" + std::to_string(::getpid()) + ".log";
     Debug::TruncateRoleLogs(clientLog.c_str());
-    const ChromeFenceReport r = RunPeer<ChromeFenceReport>(server, clientLog, RunChromeGpuFence);
+    const ChromeFenceReport r = RunPeer<ChromeFenceReport>(&server, clientLog, RunChromeGpuFence);
     ASSERT_EQ(r.initialized, 1) << r.note;
     // ANGLE offers Chrome fences only with all three, and loads the KHR names.
     EXPECT_EQ(r.fenceSyncExtension, 1);
@@ -463,3 +513,30 @@ TEST(GlamorEglSequence, ChromesGpuProcessGetsANativeFenceOfItsDmaBufWrites) {
         std::filesystem::remove(clientLog, ec);
     }
 }
+
+TEST(GlamorEglSequence, AnInprocClientIsOfferedNoSharedImagesAndAllocatesNone) {
+    PinHeadlessEgl();
+    const std::string clientLog = "/tmp/mgl-inproc-images-" + std::to_string(::getpid()) + ".log";
+    Debug::TruncateRoleLogs(clientLog.c_str());
+    const InprocReport r = RunPeer<InprocReport>(nullptr, clientLog, RunInproc);
+    ASSERT_EQ(r.initialized, 1) << r.note;
+    ASSERT_EQ(r.current, 1) << "error 0x" << std::hex << r.eglError;
+    EXPECT_EQ(r.dmabufImport, 0) << "an inproc display offered dma-buf import";
+    EXPECT_EQ(r.nativeFence, 0) << "an inproc display offered native fences";
+    EXPECT_EQ(r.externalImage, 0) << "an inproc context offered external (YUV) images";
+    EXPECT_EQ(r.allocated, 0) << "an inproc client was given a shared image";
+    std::string log;
+    for (const auto role : {Debug::LogRole::Client, Debug::LogRole::Server}) {
+        std::ifstream in(Debug::RoleLogPath(clientLog.c_str(), role));
+        log += std::string(std::istreambuf_iterator<char>(in), {});
+    }
+    EXPECT_EQ(log.find("Shared images: the server"), std::string::npos) << "the YUV probe allocated an image";
+    EXPECT_NE(log.find("shared-image allocation was refused: the client is this process"), std::string::npos)
+        << "the server did not refuse the allocation by name";
+    std::error_code ec;
+    if (!::testing::Test::HasFailure()) {
+        std::filesystem::remove(Debug::RoleLogPath(clientLog.c_str(), Debug::LogRole::Client), ec);
+        std::filesystem::remove(Debug::RoleLogPath(clientLog.c_str(), Debug::LogRole::Server), ec);
+        std::filesystem::remove(clientLog, ec);
+    }
+}

MobileGL/MG_Test/Wire/RemoteClientAdoptT0.inc

@@ -200,7 +200,7 @@ size_t CountOf(const std::string& log, const std::string& needle) {
 // The grant with the stand-ins, the store, and the map_persistent: returns the adopted pointer.
 void* AdoptOneStore(Uint8 salt, std::vector<Uint8>& seed) {
     MG_Config::Ipc.AdoptTier = 0;
-    T0::SetPeerInAnotherProcessForTest(true);
+    MobileGL::MG_Remote::Transport::SetPeerInAnotherProcessForTest(true);
     T0::SetPlatformForTest(&kFakePlatform);
     MG_Pipe::MGPipeSetResourceOps(&g_fakeOps);
     StartControlSession();
@@ -240,7 +240,7 @@ TEST(RemoteClientAdoptT0, KnobZeroOverSharedSegmentsAsksT0AndFallsBackToT2ByName
 TEST(RemoteClientAdoptT0, KnobTwoIsTodaysT2WithNoT0Line) {
     const auto child = RunInChild([] {
         MG_Config::Ipc.AdoptTier = 2;
-        T0::SetPeerInAnotherProcessForTest(true);
+        MobileGL::MG_Remote::Transport::SetPeerInAnotherProcessForTest(true);
         T0::SetPlatformForTest(&kFakePlatform);
         MG_Pipe::MGPipeSetResourceOps(&g_fakeOps);
         StartControlSession();
@@ -269,7 +269,7 @@ TEST(RemoteClientAdoptT0, KnobTwoIsTodaysT2WithNoT0Line) {
 TEST(RemoteClientAdoptT0, AnUnsetKnobFallsBackToT2QuietlyAndCountsIt) {
     const auto child = RunInChild([] {
         MG_Config::Ipc.AdoptTier = MG_Config::kAdoptTierUnset;
-        T0::SetPeerInAnotherProcessForTest(true);
+        MobileGL::MG_Remote::Transport::SetPeerInAnotherProcessForTest(true);
         MG_Pipe::MGPipeSetResourceOps(&g_fakeOps); // a backend WITH the members: the platform decides
         StartControlSession();
         if (!Srv::ServerSessionInstance().AdoptT0Asked()) ::_exit(210); // the Hello asked T0 by default
@@ -311,7 +311,7 @@ TEST(RemoteClientAdoptT0, KnobOneIsRefusedByNameAndAsksT2) {
 TEST(RemoteClientAdoptT0, NoAhardwareBufferOnThisPlatformIsT2AndTwoNamedLines) {
     const auto child = RunInChild([] {
         MG_Config::Ipc.AdoptTier = 0;
-        T0::SetPeerInAnotherProcessForTest(true);
+        MobileGL::MG_Remote::Transport::SetPeerInAnotherProcessForTest(true);
         MG_Pipe::MGPipeSetResourceOps(&g_fakeOps); // a backend WITH the members: the platform decides
         StartControlSession();
         GrantAtFirstBind();