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.

Closes bazelbuild#20658.

PiperOrigin-RevId: 595500822
Change-Id: Icf3d51bffdd004f668ba4fbbdbd5f833c20db3d9

src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java

@@ -59,7 +59,6 @@
 import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Optional;
 import java.util.SortedMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
@@ -74,7 +73,6 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner {
 
   // Since checking if sandbox is supported is expensive, we remember what we've checked.
   private static final Map<Path, Boolean> isSupportedMap = new HashMap<>();
-  private static final AtomicBoolean warnedAboutNonHermeticTmp = new AtomicBoolean();
 
   private static final AtomicBoolean warnedAboutUnsupportedModificationCheck = new AtomicBoolean();
 
@@ -205,22 +203,6 @@ private boolean useHermeticTmp() {
       return false;
     }
 
-    Optional<PathFragment> 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;
   }
 
@@ -295,6 +277,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);

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()) {

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();

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 <<EOF
+genrule(
+    name = "gen",
+    outs = ["gen.txt"],
+    cmd = """
+# Verify that /tmp is still hermetic and that the tmpfs under /tmp exists, but is empty.
+[[ ! -e "${tmp_file}" ]]
+[[ -d /tmp/tmpfs ]]
+[[ ! -e /tmp/tmpfs/data.txt ]]
+# Verify that the tmpfs on /etc exists and is empty.
+[[ -d /etc ]]
+[[ -z "\$\$(ls -A /etc)" ]]
+touch \$@
+""",
+)
+EOF
+
+  # This assumes the existence of /etc on the host system
+  bazel build --sandbox_tmpfs_path=/tmp/tmpfs --sandbox_tmpfs_path=/etc \
+    //pkg:gen || fail "build failed"
+}
+
 # The test shouldn't fail if the environment doesn't support running it.
 check_sandbox_allowed || exit 0