From fa3f6c5173712a94f9ea29e7f29584ff603d0bd6 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 22 Dec 2023 08:40:31 +0100 Subject: [PATCH] Add support for tmpfs mounts under `/tmp` with hermetic tmp This is achieved by mounting tmpfs after regular mounts in the sandbox binary as well as creating the directories at which tmpfs are mounted under the sandbox tmp directory. --- .../sandbox/LinuxSandboxedSpawnRunner.java | 27 +++++------- src/main/tools/linux-sandbox-pid1.cc | 18 ++++---- .../LinuxSandboxedSpawnRunnerTest.java | 21 ---------- src/test/shell/bazel/bazel_sandboxing_test.sh | 41 +++++++++++++++++++ 4 files changed, 60 insertions(+), 47 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java index 9c623357e66dae..ac325e383bba79 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java @@ -74,7 +74,6 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { // Since checking if sandbox is supported is expensive, we remember what we've checked. private static final Map isSupportedMap = new HashMap<>(); - private static final AtomicBoolean warnedAboutNonHermeticTmp = new AtomicBoolean(); private static final AtomicBoolean warnedAboutUnsupportedModificationCheck = new AtomicBoolean(); @@ -206,22 +205,6 @@ private boolean useHermeticTmp() { return false; } - Optional tmpfsPathUnderTmp = - getSandboxOptions().sandboxTmpfsPath.stream() - .filter(path -> path.startsWith(SLASH_TMP)) - .findFirst(); - if (tmpfsPathUnderTmp.isPresent()) { - if (warnedAboutNonHermeticTmp.compareAndSet(false, true)) { - reporter.handle( - Event.warn( - String.format( - "Falling back to non-hermetic '/tmp' in sandbox due to '%s' being a tmpfs path", - tmpfsPathUnderTmp.get()))); - } - - return false; - } - return true; } @@ -296,6 +279,16 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context createDirectoryWithinSandboxTmp(sandboxTmp, withinSandboxExecRoot); createDirectoryWithinSandboxTmp(sandboxTmp, withinSandboxWorkingDirectory); + + for (PathFragment pathFragment : getSandboxOptions().sandboxTmpfsPath) { + Path path = fileSystem.getPath(pathFragment); + if (path.startsWith(SLASH_TMP)) { + // tmpfs mount points must exist, which is usually the user's responsibility. But if the + // user requests a tmpfs mount under /tmp, we have to create it under the sandbox tmp + // directory. + createDirectoryWithinSandboxTmp(sandboxTmp, path); + } + } } SandboxOutputs outputs = helpers.getOutputs(spawn); diff --git a/src/main/tools/linux-sandbox-pid1.cc b/src/main/tools/linux-sandbox-pid1.cc index e032c689bb484f..c0626cb3b1fb74 100644 --- a/src/main/tools/linux-sandbox-pid1.cc +++ b/src/main/tools/linux-sandbox-pid1.cc @@ -271,15 +271,6 @@ static void SetupUtsNamespace() { } static void MountFilesystems() { - for (const std::string &tmpfs_dir : opt.tmpfs_dirs) { - PRINT_DEBUG("tmpfs: %s", tmpfs_dir.c_str()); - if (mount("tmpfs", tmpfs_dir.c_str(), "tmpfs", - MS_NOSUID | MS_NODEV | MS_NOATIME, nullptr) < 0) { - DIE("mount(tmpfs, %s, tmpfs, MS_NOSUID | MS_NODEV | MS_NOATIME, nullptr)", - tmpfs_dir.c_str()); - } - } - // An attempt to mount the sandbox in tmpfs will always fail, so this block is // slightly redundant with the next mount() check, but dumping the mount() // syscall is incredibly cryptic, so we explicitly check against and warn @@ -307,6 +298,15 @@ static void MountFilesystems() { } } + for (const std::string &tmpfs_dir : opt.tmpfs_dirs) { + PRINT_DEBUG("tmpfs: %s", tmpfs_dir.c_str()); + if (mount("tmpfs", tmpfs_dir.c_str(), "tmpfs", + MS_NOSUID | MS_NODEV | MS_NOATIME, nullptr) < 0) { + DIE("mount(tmpfs, %s, tmpfs, MS_NOSUID | MS_NODEV | MS_NOATIME, nullptr)", + tmpfs_dir.c_str()); + } + } + for (const std::string &writable_file : opt.writable_files) { PRINT_DEBUG("writable: %s", writable_file.c_str()); if (bind_mount_sources.find(writable_file) != bind_mount_sources.end()) { diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunnerTest.java index dd5363686efcc6..1dfbf40572d4d0 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunnerTest.java @@ -203,27 +203,6 @@ public void hermeticTmp_sandboxTmpfsOnTmp_tmpNotCreatedOrMounted() throws Except assertThat(args).doesNotContain("-m /tmp"); } - @Test - public void hermeticTmp_sandboxTmpfsUnderTmp_tmpNotCreatedOrMounted() throws Exception { - runtimeWrapper.addOptions( - "--incompatible_sandbox_hermetic_tmp", "--sandbox_tmpfs_path=/tmp/subdir"); - CommandEnvironment commandEnvironment = createCommandEnvironment(); - LinuxSandboxedSpawnRunner runner = setupSandboxAndCreateRunner(commandEnvironment); - Spawn spawn = new SpawnBuilder().build(); - SandboxedSpawn sandboxedSpawn = runner.prepareSpawn(spawn, createSpawnExecutionContext(spawn)); - - Path sandboxPath = - sandboxedSpawn.getSandboxExecRoot().getParentDirectory().getParentDirectory(); - Path hermeticTmpPath = sandboxPath.getRelative("_hermetic_tmp"); - assertThat(hermeticTmpPath.isDirectory()).isFalse(); - - assertThat(sandboxedSpawn).isInstanceOf(SymlinkedSandboxedSpawn.class); - String args = String.join(" ", sandboxedSpawn.getArguments()); - assertThat(args).contains("-w /tmp"); - assertThat(args).contains("-e /tmp"); - assertThat(args).doesNotContain("-m /tmp"); - } - private static LinuxSandboxedSpawnRunner setupSandboxAndCreateRunner( CommandEnvironment commandEnvironment) throws IOException { Path execRoot = commandEnvironment.getExecRoot(); diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh index 86fa7ad1807cd4..f74bf082fc21aa 100755 --- a/src/test/shell/bazel/bazel_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_sandboxing_test.sh @@ -423,6 +423,47 @@ EOF bazel --output_base="$tmp_output_base" shutdown } +function test_tmpfs_path_under_tmp() { + if [[ "$PLATFORM" == "darwin" ]]; then + # Tests Linux-specific functionality + return 0 + fi + + create_workspace_with_default_repos WORKSPACE + + sed -i.bak '/sandbox_tmpfs_path/d' $TEST_TMPDIR/bazelrc + + local tmp_file=$(mktemp "/tmp/bazel_tmp.XXXXXXXX") + trap "rm $tmp_file" EXIT + echo BAD > "$tmp_file" + + local tmpfs=$(mktemp -d "/tmp/bazel_tmpfs.XXXXXXXX") + trap "rm -fr $tmpfs" EXIT + echo BAD > "$tmpfs/data.txt" + + mkdir -p pkg + cat > pkg/BUILD <