diff --git a/CMakeLists.txt b/CMakeLists.txt index 78be3a55..63999257 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -304,6 +304,7 @@ set(SOURCE_FILES MobileGL/ConfigLoader.cpp MobileGL/MG_Util/Debug/Log.cpp + MobileGL/MG_Util/X11/DisplayGuard.cpp MobileGL/MG_Util/Async/JobNode.cpp MobileGL/MG_Util/Async/ShaderCompilePool.cpp diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index e1ca00a3..ef5280ae 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -8,6 +8,7 @@ #include "DirectGLES.h" #include "ContextEpoch.h" +#include "MG_Util/X11/DisplayGuard.h" #include "EGL/egl.h" #include "MG_Util/Types.h" #include "Utils.h" @@ -16769,7 +16770,7 @@ namespace MobileGL::MG_Backend::DirectGLES { static EGLint QueryDefaultX11VisualId() { #if defined(__linux__) && !defined(__ANDROID__) const char* displayName = std::getenv("DISPLAY"); - if (!displayName) { + if (!displayName || !MG_Util::X11::MayDialX11Display(displayName)) { return 0; } @@ -16816,7 +16817,7 @@ namespace MobileGL::MG_Backend::DirectGLES { return 0; } const char* displayName = std::getenv("DISPLAY"); - if (!displayName) { + if (!displayName || !MG_Util::X11::MayDialX11Display(displayName)) { return 0; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index a138113a..c8058705 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -8,6 +8,7 @@ #include "VulkanRenderer.h" #include "SubmitFencePrefix.h" +#include "MG_Util/X11/DisplayGuard.h" #include "OffscreenSurfaceRoute.h" #include "MG_Backend/DirectVulkan/SubgroupSupportPolicy.h" @@ -16720,7 +16721,10 @@ void main() { } const char* displayName = std::getenv("DISPLAY"); - auto* display = xOpenDisplay(displayName); + // Never the display this process serves (MG_Util/X11/DisplayGuard.h). + auto* display = displayName != nullptr && MG_Util::X11::MayDialX11Display(displayName) + ? xOpenDisplay(displayName) + : nullptr; if (display == nullptr) { MGLOG_F("XOpenDisplay(%s) failed while creating a Vulkan Xlib window surface; there is no usable X " "display for the requested window surface", diff --git a/MobileGL/MG_State/EGLState/Core.cpp b/MobileGL/MG_State/EGLState/Core.cpp index d5b46c3b..e4c500bc 100644 --- a/MobileGL/MG_State/EGLState/Core.cpp +++ b/MobileGL/MG_State/EGLState/Core.cpp @@ -8,6 +8,7 @@ #include "Core.h" #include +#include namespace MobileGL { namespace MG_State { @@ -33,7 +34,9 @@ namespace MobileGL { EGLint QueryDefaultX11VisualId() { #if defined(__linux__) && !defined(__ANDROID__) const char* displayName = std::getenv("DISPLAY"); - if (!displayName) { + // Never the display this process serves: an X server drawing with MobileGL + // (Xwayland's glamor) would wait on its own socket (MG_Util/X11/DisplayGuard.h). + if (!displayName || !MG_Util::X11::MayDialX11Display(displayName)) { return 0; } diff --git a/MobileGL/MG_Test/Util/CMakeLists.txt b/MobileGL/MG_Test/Util/CMakeLists.txt index d4ccc8fa..c3b600db 100644 --- a/MobileGL/MG_Test/Util/CMakeLists.txt +++ b/MobileGL/MG_Test/Util/CMakeLists.txt @@ -89,7 +89,26 @@ target_link_libraries( ${LINK_LIBRARIES} ) +# The X-connection safety rule: MobileGL never dials an X display its own process serves (the +# Xwayland-on-glamor deadlock), with the /proc facts injected and once for real in a child. +add_executable( + X11DisplayGuardTest + X11DisplayGuardTest.cpp +) + +target_include_directories(X11DisplayGuardTest PRIVATE + ${MGL_ROOT}/include + ${MGL_ROOT}/MobileGL +) + +target_link_libraries( + X11DisplayGuardTest PRIVATE + GTest::gtest_main + ${LINK_LIBRARIES} +) + include(GoogleTest) +gtest_discover_tests(X11DisplayGuardTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) gtest_discover_tests(JobNodeTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) gtest_discover_tests(LogLevelTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) gtest_discover_tests(PipeStatsTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) diff --git a/MobileGL/MG_Test/Util/X11DisplayGuardTest.cpp b/MobileGL/MG_Test/Util/X11DisplayGuardTest.cpp new file mode 100644 index 00000000..31d35362 --- /dev/null +++ b/MobileGL/MG_Test/Util/X11DisplayGuardTest.cpp @@ -0,0 +1,165 @@ +// MobileGL - MobileGL/MG_Test/Util/X11DisplayGuardTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// The X-connection safety rule (MG_Util/X11/DisplayGuard.h): MobileGL never opens a connection to a +// display its own process serves. The last case is the deadlock itself: a process that listens on +// an X display's socket, as Xwayland does, creates an EGL display with DISPLAY naming that socket. +// Before the guard EGLState dialed it to pick a visual and waited forever for a setup reply only +// the blocked process could send. + +#include +#include + +#include + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +using namespace MobileGL; +namespace X = MobileGL::MG_Util::X11; + +namespace { + // /proc/net/unix as Xwayland's process sees it under KWin: the two listening sockets KWin bound + // and handed over with -listenfd, plus connected ones. + const char* kProcNetUnix = + "Num RefCount Protocol Flags Type St Inode Path\n" + "0000000000000000: 00000002 00000000 00010000 0001 01 41001 @/tmp/.X11-unix/X0\n" + "0000000000000000: 00000002 00000000 00010000 0001 01 41002 /tmp/.X11-unix/X0\n" + "0000000000000000: 00000003 00000000 00000000 0001 03 41003 /tmp/.X11-unix/X0\n" + "0000000000000000: 00000002 00000000 00010000 0001 01 41004 /run/user/1000/wayland-0\n" + "0000000000000000: 00000003 00000000 00000000 0001 03 41005\n" + "0000000000000000: 00000002 00000000 00010000 0001 01 41006 @/tmp/.X11-unix/X1\n"; +} // namespace + +TEST(X11DisplayGuard, DisplayNamesYieldTheirNumber) { + EXPECT_EQ(X::ParseDisplayNumber(":0"), 0); + EXPECT_EQ(X::ParseDisplayNumber(":1.0"), 1); + EXPECT_EQ(X::ParseDisplayNumber("unix:2"), 2); + EXPECT_EQ(X::ParseDisplayNumber("localhost:10.2"), 10); + EXPECT_EQ(X::ParseDisplayNumber("[::1]:3"), 3); + EXPECT_EQ(X::ParseDisplayNumber("/tmp/.X11-unix/X4"), 4); + EXPECT_EQ(X::ParseDisplayNumber("@/tmp/.X11-unix/X5"), 5); + EXPECT_EQ(X::ParseDisplayNumber(""), -1); + EXPECT_EQ(X::ParseDisplayNumber(nullptr), -1); + EXPECT_EQ(X::ParseDisplayNumber("wayland-0"), -1); + EXPECT_EQ(X::ParseDisplayNumber(":x"), -1); +} + +TEST(X11DisplayGuard, OnlyListeningSocketsOfThatDisplayCount) { + const Vector zero = X::ListeningDisplaySocketInodes(kProcNetUnix, 0); + ASSERT_EQ(zero.size(), 2u); + EXPECT_EQ(zero[0], 41001u); // abstract + EXPECT_EQ(zero[1], 41002u); // path; 41003 is a connection to it, not a listener + EXPECT_EQ(X::ListeningDisplaySocketInodes(kProcNetUnix, 1), Vector{41006u}); + EXPECT_TRUE(X::ListeningDisplaySocketInodes(kProcNetUnix, 7).empty()); +} + +TEST(X11DisplayGuard, AnXServerNeverDialsItsOwnDisplay) { + // Xwayland holding display 0's listening socket. + X::ProcessFacts xwayland{"Xwayland", kProcNetUnix, {7, 41002, 9}}; + EXPECT_FALSE(X::MayDial(":0", xwayland, nullptr)); + // By executable alone (a socket held through a path /proc does not show): still refused. + X::ProcessFacts xwaylandNoSocket{"Xwayland", kProcNetUnix, {}}; + EXPECT_FALSE(X::MayDial(":0", xwaylandNoSocket, nullptr)); + // A process that is not named like an X server but holds the listener serves it too. + X::ProcessFacts renamed{"my-x-server", kProcNetUnix, {41001}}; + EXPECT_FALSE(X::MayDial(":0", renamed, nullptr)); + EXPECT_TRUE(X::MayDial(":1", renamed, nullptr)) << "display 1 is someone else's"; +} + +TEST(X11DisplayGuard, AnOrdinaryClientDialsUnlessForbidden) { + X::ProcessFacts client{"glxgears", kProcNetUnix, {41003, 41005}}; // connected, not listening + EXPECT_TRUE(X::MayDial(":0", client, nullptr)); + EXPECT_TRUE(X::MayDial(":0", client, "1")); + EXPECT_FALSE(X::MayDial(":0", client, "0")); + EXPECT_FALSE(X::MayDial(":0", client, "off")); + EXPECT_FALSE(X::MayDial(nullptr, client, nullptr)); + EXPECT_FALSE(X::MayDial("", client, nullptr)); + EXPECT_TRUE(X::MayDial("weird-name", client, nullptr)) << "no number: only an X server refuses"; + EXPECT_TRUE(X::IsXServerProgram("Xorg")); + EXPECT_TRUE(X::IsXServerProgram("Xvfb")); + EXPECT_FALSE(X::IsXServerProgram("Xclock")); +} + +namespace { + // A listening abstract socket named like display `number`'s, as an X server (or KWin, for + // Xwayland's -listenfd) binds it. + int ListenAsDisplay(int number) { + const int fd = ::socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0); + if (fd < 0) return -1; + sockaddr_un addr{}; + addr.sun_family = AF_UNIX; + const std::string name = "/tmp/.X11-unix/X" + std::to_string(number); + std::memcpy(addr.sun_path + 1, name.data(), name.size()); // abstract: leading NUL + const socklen_t length = static_cast(offsetof(sockaddr_un, sun_path) + 1 + name.size()); + if (::bind(fd, reinterpret_cast(&addr), length) != 0 || ::listen(fd, 4) != 0) { + ::close(fd); + return -1; + } + return fd; + } + + int UnusedDisplayNumber() { return 3000 + static_cast(::getpid() % 2000); } +} // namespace + +TEST(X11DisplayGuard, ThisProcessServesADisplayWhileItListensOnItsSocket) { + const int number = UnusedDisplayNumber(); + const std::string name = ":" + std::to_string(number); + EXPECT_TRUE(X::MayDialX11Display(name.c_str())); + const int listener = ListenAsDisplay(number); + ASSERT_GE(listener, 0) << std::strerror(errno); + EXPECT_FALSE(X::MayDialX11Display(name.c_str())); + ::close(listener); + EXPECT_TRUE(X::MayDialX11Display(name.c_str())); +} + +// THE DEADLOCK. In a child (a hang must not take the suite with it): listen like an X server, +// point DISPLAY at ourselves, and create the default EGL display - which used to XOpenDisplay +// DISPLAY for the configs' native visual and block in the connection setup forever. +TEST(X11DisplayGuard, AnEglDisplayInAnXServerDoesNotDialItself) { + const int number = UnusedDisplayNumber() + 1; + int fds[2]; + ASSERT_EQ(::pipe(fds), 0); + const pid_t pid = ::fork(); + ASSERT_GE(pid, 0); + if (pid == 0) { + ::close(fds[0]); + const int listener = ListenAsDisplay(number); + if (listener < 0) ::_exit(3); + const std::string name = ":" + std::to_string(number); + ::setenv("DISPLAY", name.c_str(), 1); + ::unsetenv("MOBILEGL_X11_DIAL"); + { + MG_State::EGLState::EGLContext state; + const EGLDisplay display = state.GetDisplay(EGL_DEFAULT_DISPLAY); + const char ok = display != EGL_NO_DISPLAY ? 'y' : 'n'; + (void)!::write(fds[1], &ok, 1); + } + ::_exit(0); + } + ::close(fds[1]); + pollfd p{fds[0], POLLIN, 0}; + const int ready = ::poll(&p, 1, 15000); + char got = 0; + if (ready > 0) (void)!::read(fds[0], &got, 1); + if (ready <= 0) ::kill(pid, SIGKILL); + int status = 0; + ::waitpid(pid, &status, 0); + ::close(fds[0]); + ASSERT_GT(ready, 0) << "eglGetDisplay blocked: MobileGL dialed the X display its own process serves"; + EXPECT_EQ(got, 'y'); +} diff --git a/MobileGL/MG_Util/X11/DisplayGuard.cpp b/MobileGL/MG_Util/X11/DisplayGuard.cpp new file mode 100644 index 00000000..4beab4b3 --- /dev/null +++ b/MobileGL/MG_Util/X11/DisplayGuard.cpp @@ -0,0 +1,180 @@ +// MobileGL - MobileGL/MG_Util/X11/DisplayGuard.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#include "DisplayGuard.h" + +#include + +#include +#include +#include +#include +#include + +#if defined(__linux__) +#include +#include +#endif + +namespace MobileGL::MG_Util::X11 { + namespace { + constexpr const char* kSocketDirectory = "/tmp/.X11-unix/X"; + // __SO_ACCEPTCON in /proc/net/unix's Flags column: the socket listens. + constexpr unsigned long kAcceptConnectionsFlag = 0x00010000ul; + + Bool AllDigits(const char* begin, const char* end) { + if (begin == end) return false; + for (const char* p = begin; p != end; ++p) { + if (*p < '0' || *p > '9') return false; + } + return true; + } + + Int32 ToNumber(const char* begin, const char* end) { + if (!AllDigits(begin, end) || end - begin > 6) return -1; + Int32 value = 0; + for (const char* p = begin; p != end; ++p) value = value * 10 + (*p - '0'); + return value; + } + + String ReadWhole(const char* path) { + std::ifstream in(path, std::ios::binary); + if (!in) return {}; + std::stringstream all; + all << in.rdbuf(); + return all.str(); + } + + Bool Forbidden(const char* value) { + if (value == nullptr) return false; + return std::strcmp(value, "0") == 0 || std::strcmp(value, "false") == 0 || std::strcmp(value, "off") == 0 || + std::strcmp(value, "no") == 0; + } + } // namespace + + Int32 ParseDisplayNumber(const char* name) { + if (name == nullptr || name[0] == '\0') return -1; + const SizeT length = std::strlen(name); + // A socket path (launchd-style names, and how some tools spell a local display). + const SizeT dirLength = std::strlen(kSocketDirectory); + const char* path = name[0] == '@' ? name + 1 : name; + if (std::strncmp(path, kSocketDirectory, dirLength) == 0) { + const char* digits = path + dirLength; + const char* end = digits; + while (*end >= '0' && *end <= '9') ++end; + return ToNumber(digits, end); + } + // [protocol/][host]:display[.screen]: the LAST colon (an IPv6 host has colons of its own). + const char* colon = std::strrchr(name, ':'); + if (colon == nullptr) return -1; + const char* digits = colon + 1; + const char* end = digits; + while (*end >= '0' && *end <= '9') ++end; + if (*end != '\0' && *end != '.') return -1; + (void)length; + return ToNumber(digits, end); + } + + Bool IsXServerProgram(const String& basename) { + static const char* const kServers[] = {"Xwayland", "Xorg", "Xorg.bin", "X", "Xvfb", "Xephyr", + "Xnest", "Xvnc", "Xwin", "XWin", "Xquartz", "Xfake"}; + for (const char* server : kServers) { + if (basename == server) return true; + } + return false; + } + + Vector ListeningDisplaySocketInodes(const String& procNetUnix, Int32 number) { + Vector inodes; + if (number < 0) return inodes; + const String wanted = String(kSocketDirectory) + std::to_string(number); + std::istringstream lines(procNetUnix); + String line; + Bool header = true; + while (std::getline(lines, line)) { + if (header) { // "Num RefCount Protocol Flags Type St Inode Path" + header = false; + if (line.find("Inode") != String::npos) continue; + } + std::istringstream fields(line); + String num, refCount, protocol, flags, type, state, inode, path; + if (!(fields >> num >> refCount >> protocol >> flags >> type >> state >> inode)) continue; + if (!(fields >> path)) continue; // unbound + const unsigned long flagBits = std::strtoul(flags.c_str(), nullptr, 16); + if ((flagBits & kAcceptConnectionsFlag) == 0) continue; + const String bare = !path.empty() && path[0] == '@' ? path.substr(1) : path; + if (bare != wanted) continue; + inodes.push_back(std::strtoull(inode.c_str(), nullptr, 10)); + } + return inodes; + } + + Bool ServesDisplay(const ProcessFacts& facts, Int32 number) { + if (number < 0) return false; + for (const Uint64 listening : ListeningDisplaySocketInodes(facts.ProcNetUnix, number)) { + for (const Uint64 own : facts.OwnSocketInodes) { + if (own == listening) return true; + } + } + return false; + } + + Bool MayDial(const char* displayName, const ProcessFacts& facts, const char* overrideValue) { + if (Forbidden(overrideValue)) return false; + if (displayName == nullptr || displayName[0] == '\0') return false; + // An X server never dials a display: the one it would reach first is likely its own, and + // nothing it draws needs another one. + if (IsXServerProgram(facts.ExeBasename)) return false; + const Int32 number = ParseDisplayNumber(displayName); + return number < 0 || !ServesDisplay(facts, number); + } + + ProcessFacts ReadProcessFacts() { + ProcessFacts facts; +#if defined(__linux__) + char exe[4096] = {}; + const ssize_t length = ::readlink("/proc/self/exe", exe, sizeof(exe) - 1); + if (length > 0) { + exe[length] = '\0'; + const char* slash = std::strrchr(exe, '/'); + facts.ExeBasename = slash != nullptr ? slash + 1 : exe; + } + facts.ProcNetUnix = ReadWhole("/proc/self/net/unix"); + if (DIR* fds = ::opendir("/proc/self/fd")) { + while (const dirent* entry = ::readdir(fds)) { + if (entry->d_name[0] == '.') continue; + const String link = String("/proc/self/fd/") + entry->d_name; + char target[128] = {}; + const ssize_t n = ::readlink(link.c_str(), target, sizeof(target) - 1); + if (n <= 0) continue; + target[n] = '\0'; + // "socket:[12345]" + if (std::strncmp(target, "socket:[", 8) != 0) continue; + facts.OwnSocketInodes.push_back(std::strtoull(target + 8, nullptr, 10)); + } + ::closedir(fds); + } +#endif + return facts; + } + + Bool MayDialX11Display(const char* displayName) { + const char* overrideValue = std::getenv("MOBILEGL_X11_DIAL"); + if (displayName == nullptr || displayName[0] == '\0') return false; + const Bool allowed = MayDial(displayName, ReadProcessFacts(), overrideValue); + if (!allowed) { + static std::atomic logged{false}; + if (!logged.exchange(true)) { + MGLOG_I("X11: not opening a connection to display '%s' - this process serves it (an X server), or " + "MOBILEGL_X11_DIAL forbids it", + displayName); + } + } + return allowed; + } +} // namespace MobileGL::MG_Util::X11 diff --git a/MobileGL/MG_Util/X11/DisplayGuard.h b/MobileGL/MG_Util/X11/DisplayGuard.h new file mode 100644 index 00000000..bf0580d2 --- /dev/null +++ b/MobileGL/MG_Util/X11/DisplayGuard.h @@ -0,0 +1,59 @@ +// MobileGL - MobileGL/MG_Util/X11/DisplayGuard.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#pragma once + +// MOBILEGL NEVER DIALS AN X DISPLAY ITS OWN PROCESS SERVES. +// +// A rootless X server that draws with MobileGL (Xwayland's glamor) loads this library into the X +// server itself. Anything here that opens a connection to $DISPLAY then connects to the very +// process that is blocked inside the call - which can never answer: the X server deadlocks on its +// own socket. So every place that would open an X connection asks MayDialX11Display first. +// +// "Serves" is decided by facts about the process, not by who called: it holds a LISTENING unix +// socket bound to that display's name (/tmp/.X11-unix/X, as a path or in the abstract +// namespace - an X server started with -listenfd holds the ones its compositor bound), or its +// executable is an X server's. MOBILEGL_X11_DIAL=0 forbids every dial. +// +// The pure pieces take their inputs (the /proc/net/unix text, the process's socket inodes, the +// executable name) so a host test pins them without being an X server. + +#include + +namespace MobileGL::MG_Util::X11 { + // The display number of an X display name - ":0", ":1.0", "unix:2", "host:3.1", + // "/tmp/.X11-unix/X4" - or -1 when the name has none. + Int32 ParseDisplayNumber(const char* name); + + // Whether an executable's base name is an X server's (Xwayland, Xorg, Xvfb, ...). + Bool IsXServerProgram(const String& basename); + + // The inodes of the LISTENING sockets in a /proc/net/unix table bound to display `number`'s + // socket name, as a path or abstract (leading '@'). + Vector ListeningDisplaySocketInodes(const String& procNetUnix, Int32 number); + + struct ProcessFacts { + String ExeBasename; + String ProcNetUnix; + Vector OwnSocketInodes; // inodes of the sockets in the process's descriptor table + }; + + // Whether the process the facts describe serves display `number`. + Bool ServesDisplay(const ProcessFacts& facts, Int32 number); + + // The decision: false for no name, a forbidding override ("0"/"false"/"off"), or a display this + // process serves; a name with no number is dialed only by a process that is not an X server. + Bool MayDial(const char* displayName, const ProcessFacts& facts, const char* overrideValue); + + // The real process's facts, read from /proc. + ProcessFacts ReadProcessFacts(); + + // MayDial for this process (facts read on each call: rare - a display creation, a window + // surface - and a process may start serving later). Logs a refusal once. + Bool MayDialX11Display(const char* displayName); +} // namespace MobileGL::MG_Util::X11