diff --git a/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp b/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp index 45375ee4..cc14fcab 100644 --- a/MobileGL/MG_Impl/EGLImpl/EGLImpl.cpp +++ b/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 diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index f756292a..59b47ccb 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/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 diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 6534f3ad..7c6a5c65 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/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(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 diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index 13276549..a110711e 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/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) { diff --git a/MobileGL/MG_Remote/Transport/AdoptT0.cpp b/MobileGL/MG_Remote/Transport/AdoptT0.cpp index f855e7f6..c3b43c17 100644 --- a/MobileGL/MG_Remote/Transport/AdoptT0.cpp +++ b/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 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); diff --git a/MobileGL/MG_Remote/Transport/AdoptT0.h b/MobileGL/MG_Remote/Transport/AdoptT0.h index bf223653..5200c6d6 100644 --- a/MobileGL/MG_Remote/Transport/AdoptT0.h +++ b/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 ---------------------------------------------------------------------- diff --git a/MobileGL/MG_Remote/Transport/ITransport.h b/MobileGL/MG_Remote/Transport/ITransport.h index 723ab538..50e64950 100644 --- a/MobileGL/MG_Remote/Transport/ITransport.h +++ b/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 diff --git a/MobileGL/MG_Remote/Transport/InProcessTransport.cpp b/MobileGL/MG_Remote/Transport/InProcessTransport.cpp index e3253913..255282c6 100644 --- a/MobileGL/MG_Remote/Transport/InProcessTransport.cpp +++ b/MobileGL/MG_Remote/Transport/InProcessTransport.cpp @@ -13,6 +13,7 @@ #include +#include #include #include #include @@ -290,4 +291,17 @@ namespace MobileGL::MG_Remote::Transport { Doorbell& InProcessTransport::SelfDoorbell() { return m_channel->Bell(m_endpoint); } + namespace { + std::atomic 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 diff --git a/MobileGL/MG_Test/Wire/GlamorEglSequenceTest.cpp b/MobileGL/MG_Test/Wire/GlamorEglSequenceTest.cpp index 1fd0ebbb..780d2946 100644 --- a/MobileGL/MG_Test/Wire/GlamorEglSequenceTest.cpp +++ b/MobileGL/MG_Test/Wire/GlamorEglSequenceTest.cpp @@ -32,6 +32,9 @@ #include #include +#include +#include + 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 - 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(server, clientLog, RunGlamor); + const GlamorReport r = RunPeer(&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(server, clientLog, RunChromeGpuFence); + const ChromeFenceReport r = RunPeer(&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(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(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); + } +} diff --git a/MobileGL/MG_Test/Wire/RemoteClientAdoptT0.inc b/MobileGL/MG_Test/Wire/RemoteClientAdoptT0.inc index 56abbc4c..7e39191b 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientAdoptT0.inc +++ b/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& 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();