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 <