diff --git a/.github/workflows/core.yml b/.github/workflows/core.yml index 89b65713d..555e25705 100644 --- a/.github/workflows/core.yml +++ b/.github/workflows/core.yml @@ -50,6 +50,9 @@ jobs: python3 manager/tools/check_translations.py manager/src/main/res python3 manager/tools/check_translations.py daemon/src/main/res + - name: Test dex2oat recursion guard + run: python3 dex2oat/tests/test_wrapper_recursion.py + # A missing secret used to be skipped over in silence, and the build carried on to publish a # canary or a tag signed with the debug key — which the daemon's InstallerVerifier rejects, # and which nobody notices until someone tries to install the manager. On this repository the diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/env/Dex2OatServer.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/env/Dex2OatServer.kt index 7db27300f..a41147a4d 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/env/Dex2OatServer.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/env/Dex2OatServer.kt @@ -34,6 +34,7 @@ object Dex2OatServer { private val dex2oatArray = arrayOfNulls(6) private val fdArray = arrayOfNulls(6) + private var compilerOpenFailed = false @Volatile var compatibility = DEX2OAT_OK @@ -181,12 +182,27 @@ object Dex2OatServer { if (index != -1 && dex2oatArray[index] == null) { dex2oatArray[index] = path - fdArray[index] = Os.open(path, OsConstants.O_RDONLY, 0) + val fd = Os.open(path, OsConstants.O_RDONLY, 0) + try { + // A restarted daemon can inherit the previous daemon's bind mounts. Validate + // the opened FD, not just the path, before treating it as the stock compiler. + val candidate = Os.fstat(fd) + val wrapper = Os.stat(if (is32Bit) WRAPPER32 else WRAPPER64) + check(candidate.st_dev != wrapper.st_dev || candidate.st_ino != wrapper.st_ino) { + "Refusing wrapper as stock compiler: $path" + } + fdArray[index] = fd + } finally { + if (fdArray[index] !== fd) Os.close(fd) + } Log.i(TAG, "Detected $path -> Assigned Index $index") } } } - .onFailure { dex2oatArray[dex2oatArray.indexOf(path)] = null } + .onFailure { + compilerOpenFailed = true + Log.e(TAG, "Cannot open stock compiler $path", it) + } } private fun notMounted(): Boolean { @@ -210,6 +226,19 @@ object Dex2OatServer { } fun start() { + if (compilerOpenFailed) { + // Keep the paths for cleanup, but never serve suspect descriptors. doMount(false) also + // restores the no-inline property used when the wrapper is unavailable. Do not start + // the SELinux observer: it could otherwise re-enable these mounts later. + doMount(false) + fdArray.indices.forEach { i -> + fdArray[i]?.let { fd -> runCatching { Os.close(fd) } } + fdArray[i] = null + } + compatibility = DEX2OAT_MOUNT_FAILED + return + } + if (notMounted()) { doMount(true) if (notMounted()) { diff --git a/dex2oat/src/main/cpp/dex2oat.cpp b/dex2oat/src/main/cpp/dex2oat.cpp index 32719bea7..2de1b59f8 100644 --- a/dex2oat/src/main/cpp/dex2oat.cpp +++ b/dex2oat/src/main/cpp/dex2oat.cpp @@ -26,6 +26,7 @@ extern "C" char **environ; namespace { constexpr char kSockName[] = "5291374ceda0aef7c5d86cd2a4f6a3ac"; +constexpr char kWrapperPidEnv[] = "VECTOR_DEX2OAT_WRAPPER_PID"; /** * Calculates a vector ID based on architecture and debug status. @@ -109,6 +110,20 @@ void write_int(int fd, int val) { } // namespace int main(int argc, char **argv) { + // execve preserves the PID. If the daemon accidentally sent a wrapper FD instead of the + // stock compiler, stop before receiving more FDs or copying another hook library. A PID + // marker (rather than a boolean) allows a compiler child to inherit the environment. + const auto pid = std::to_string(getpid()); + const char *previous_pid = getenv(kWrapperPidEnv); + if (previous_pid && pid == previous_pid) { + LOGE("Refusing recursive dex2oat wrapper execution; check for stale wrapper mounts"); + return 1; + } + if (setenv(kWrapperPidEnv, pid.c_str(), 1) != 0) { + PLOGE("failed to set dex2oat wrapper recursion guard"); + return 1; + } + LOGD("dex2oat wrapper ppid=%d", getppid()); // Prepare Unix domain socket address (Abstract Namespace) diff --git a/dex2oat/tests/test_wrapper_recursion.py b/dex2oat/tests/test_wrapper_recursion.py new file mode 100644 index 000000000..00a82e1e5 --- /dev/null +++ b/dex2oat/tests/test_wrapper_recursion.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +"""Linux host regression test: compile the real wrapper and emulate its FD server. + +Run with python3 dex2oat/tests/test_wrapper_recursion.py (requires g++). +Only Android logging and the Android linker path are replaced for host execution. +No root, mounts, Android device, or installed Vector instance is needed. +""" + +import array +import os +from pathlib import Path +import socket +import struct +import subprocess +import tempfile +import threading +import unittest +import uuid + + +class WrapperRecursionTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.temp = tempfile.TemporaryDirectory() + cls.addClassCleanup(cls.temp.cleanup) + cls.root = Path(cls.temp.name) + cls.address = "vector-test-" + uuid.uuid4().hex + cls.wrapper = cls.root / "wrapper" + cls.stock = cls.root / "stock" + cls.hook = cls.root / "hook.so" + linker = cls.root / "linker" + + (cls.root / "logging.h").write_text( + '#include \n#include \n' + '#define LOGD(...) ((void)0)\n' + '#define LOGI(...) ((void)0)\n' + '#define LOGE(...) fprintf(stderr, __VA_ARGS__)\n' + '#define PLOGE(...) fprintf(stderr, __VA_ARGS__)\n' + ) + source = (Path(__file__).resolve().parents[1] / "src/main/cpp/dex2oat.cpp").read_text() + source = source.replace("5291374ceda0aef7c5d86cd2a4f6a3ac", cls.address) + for name in ("linker64", "linker"): + source = source.replace("/apex/com.android.runtime/bin/" + name, str(linker)) + (cls.root / "wrapper.cpp").write_text(source) + (cls.root / "linker.cpp").write_text( + '#include \nint main(int, char **argv) { execv(argv[1], argv + 1); return 99; }\n' + ) + (cls.root / "stock.cpp").write_text( + '#include \nint main() { puts("stock compiler reached"); return 0; }\n' + ) + (cls.root / "hook.cpp").write_text('extern "C" void test_hook() {}\n') + for name in ("wrapper", "linker", "stock"): + subprocess.run( + ["g++", "-std=c++20", "-Wall", "-Wextra", str(cls.root / (name + ".cpp")), + "-o", str(cls.root / name)], check=True + ) + subprocess.run( + ["g++", "-shared", "-fPIC", str(cls.root / "hook.cpp"), "-o", str(cls.hook)], + check=True + ) + + def run_wrapper(self, recursive=False, inherited_marker=False): + requests = [] + errors = [] + stop = threading.Event() + with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as server: + server.bind("\0" + self.address) + server.listen() + server.settimeout(0.1) + + def serve(): + while not stop.is_set(): + try: + client, _ = server.accept() + except socket.timeout: + continue + try: + with client: + client.settimeout(1) + command = client.recv(4, socket.MSG_WAITALL) + index = struct.unpack("i", command)[0] + requests.append(index) + path = self.hook if index >= 4 else ( + self.wrapper if recursive else self.stock + ) + fd = os.open(path, os.O_RDONLY) + try: + client.sendmsg([b"\x01"], [ + (socket.SOL_SOCKET, socket.SCM_RIGHTS, array.array("i", [fd])) + ]) + client.sendall(struct.pack("i", 1)) + finally: + os.close(fd) + except Exception as error: + errors.append(error) + return + + thread = threading.Thread(target=serve, daemon=True) + thread.start() + env = os.environ.copy() + env.pop("VECTOR_DEX2OAT_WRAPPER_PID", None) + if inherited_marker: + env["VECTOR_DEX2OAT_WRAPPER_PID"] = str(os.getpid()) + try: + result = subprocess.run( + [str(self.wrapper)], env=env, capture_output=True, text=True, timeout=5 + ) + finally: + stop.set() + thread.join(timeout=2) + self.assertFalse(thread.is_alive()) + self.assertEqual(errors, []) + return result, requests + + def test_stock_compiler_still_runs(self): + result, requests = self.run_wrapper() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("stock compiler reached", result.stdout) + self.assertEqual(len(requests), 2) + + def test_wrapper_fd_stops_before_requesting_more_descriptors(self): + result, requests = self.run_wrapper(recursive=True) + self.assertEqual(result.returncode, 1, result.stderr) + self.assertIn("Refusing recursive dex2oat wrapper execution", result.stderr) + self.assertEqual(len(requests), 2) + + def test_parent_marker_does_not_block_child_compiler(self): + result, requests = self.run_wrapper(inherited_marker=True) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("stock compiler reached", result.stdout) + self.assertEqual(len(requests), 2) + + +if __name__ == "__main__": + unittest.main()