diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java index 0a142c167960e8..cf5fadc1ea8f0a 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java @@ -310,9 +310,13 @@ private boolean wasTimeout(Duration timeout, Duration wallTime) { /** * Gets the list of directories that the spawn will assume to be writable. * + * @param sandboxExecRoot the exec root of the sandbox from the point of view of the Bazel process + * @param withinSandboxExecRoot the exec root from the point of view of the sandboxed processes + * @param env the environment of the sandboxed processes * @throws IOException because we might resolve symlinks, which throws {@link IOException}. */ - protected ImmutableSet getWritableDirs(Path sandboxExecRoot, Map env) + protected ImmutableSet getWritableDirs( + Path sandboxExecRoot, Path withinSandboxExecRoot, Map env) throws IOException { // We have to make the TEST_TMPDIR directory writable if it is specified. ImmutableSet.Builder writablePaths = ImmutableSet.builder(); @@ -320,7 +324,7 @@ protected ImmutableSet getWritableDirs(Path sandboxExecRoot, Map writableDirs = new HashSet<>(alwaysWritableDirs); - ImmutableSet extraWritableDirs = getWritableDirs(sandboxExecRoot, environment); + ImmutableSet extraWritableDirs = + getWritableDirs(sandboxExecRoot, sandboxExecRoot, environment); writableDirs.addAll(extraWritableDirs); SandboxInputs inputs = diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java index 756617cba1a36d..d2233a9e8a5d14 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilder.java @@ -14,9 +14,9 @@ package com.google.devtools.build.lib.sandbox; +import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.vfs.Path; @@ -32,6 +32,20 @@ * linux-sandbox} tool. */ public class LinuxSandboxCommandLineBuilder { + /** A bind mount that needs to be present when the sandboxed command runs. */ + @AutoValue + public abstract static class BindMount { + public static BindMount of(Path mountPoint, Path source) { + return new AutoValue_LinuxSandboxCommandLineBuilder_BindMount(mountPoint, source); + } + + /** "target" in mount(2) */ + public abstract Path getMountPoint(); + + /** "source" in mount(2) */ + public abstract Path getContent(); + } + private final Path linuxSandboxPath; private final List commandArguments; private Path hermeticSandboxPath; @@ -43,7 +57,7 @@ public class LinuxSandboxCommandLineBuilder { private Path stderrPath; private Set writableFilesAndDirectories = ImmutableSet.of(); private ImmutableSet tmpfsDirectories = ImmutableSet.of(); - private Map bindMounts = ImmutableMap.of(); + private List bindMounts = ImmutableList.of(); private Path statisticsPath; private boolean useFakeHostname = false; private boolean createNetworkNamespace = false; @@ -140,7 +154,7 @@ public LinuxSandboxCommandLineBuilder setTmpfsDirectories( * if any. */ @CanIgnoreReturnValue - public LinuxSandboxCommandLineBuilder setBindMounts(Map bindMounts) { + public LinuxSandboxCommandLineBuilder setBindMounts(List bindMounts) { this.bindMounts = bindMounts; return this; } @@ -248,12 +262,11 @@ public ImmutableList build() { for (PathFragment tmpfsPath : tmpfsDirectories) { commandLineBuilder.add("-e", tmpfsPath.getPathString()); } - for (Path bindMountTarget : bindMounts.keySet()) { - Path bindMountSource = bindMounts.get(bindMountTarget); - commandLineBuilder.add("-M", bindMountSource.getPathString()); + for (BindMount bindMount : bindMounts) { + commandLineBuilder.add("-M", bindMount.getContent().getPathString()); // The file is mounted in a custom location inside the sandbox. - if (!bindMountSource.equals(bindMountTarget)) { - commandLineBuilder.add("-m", bindMountTarget.getPathString()); + if (!bindMount.getContent().equals(bindMount.getMountPoint())) { + commandLineBuilder.add("-m", bindMount.getMountPoint().getPathString()); } } if (statisticsPath != null) { 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 7914061ad1969b..3fae3b07923c84 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 @@ -40,6 +40,7 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.BindMount; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; import com.google.devtools.build.lib.server.FailureDetails.Sandbox.Code; @@ -67,6 +68,11 @@ /** Spawn runner that uses linux sandboxing APIs to execute a local subprocess. */ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { + private static final PathFragment SLASH_TMP = PathFragment.create("/tmp"); + private static final PathFragment BAZEL_EXECROOT = PathFragment.create("bazel-execroot"); + private static final PathFragment BAZEL_WORKING_DIRECTORY = + PathFragment.create("bazel-working-directory"); + private static final PathFragment BAZEL_SOURCE_ROOTS = PathFragment.create("bazel-source-roots"); // Since checking if sandbox is supported is expensive, we remember what we've checked. private static final Map isSupportedMap = new HashMap<>(); @@ -182,73 +188,150 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS this.packageRoots = cmdEnv.getPackageLocator().getPathEntries(); } + private void createDirectoryWithinSandboxTmp(Path sandboxTmp, Path withinSandboxDirectory) + throws IOException { + PathFragment withinTmp = withinSandboxDirectory.asFragment().relativeTo(SLASH_TMP); + sandboxTmp.getRelative(withinTmp).createDirectoryAndParents(); + } + + private boolean useHermeticTmp() { + if (!getSandboxOptions().sandboxHermeticTmp) { + // No hermetic /tmp requested, so let's not do it + return false; + } + + boolean tmpExplicitlyBindMounted = + getSandboxOptions().sandboxAdditionalMounts.stream() + .anyMatch(e -> e.getKey().equals("/tmp")); + if (tmpExplicitlyBindMounted) { + if (warnedAboutNonHermeticTmp.compareAndSet(false, true)) { + reporter.handle( + Event.warn( + "Falling back to non-hermetic '/tmp' in because a bind mount of '/tmp' is" + + " explicitly requested")); + } + + return false; + } + + if (getSandboxOptions().sandboxTmpfsPath.contains(SLASH_TMP)) { + if (warnedAboutNonHermeticTmp.compareAndSet(false, true)) { + reporter.handle( + Event.warn( + "Both hermetic '/tmp' and an explicit tmpfs mount on '/tmp' is requested, using" + + " tmpfs")); + } + + return false; + } + + Optional tmpfsPathUnderTmp = + getSandboxOptions().sandboxTmpfsPath.stream() + .filter(path -> path.startsWith(SLASH_TMP)) + .findFirst(); + if (!tmpfsPathUnderTmp.isEmpty()) { + 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; + } + @Override protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context) throws IOException, ForbiddenActionInputException, ExecException, InterruptedException { + // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like + // the normal execroot does. + String workspaceName = execRoot.getBaseName(); + // Each invocation of "exec" gets its own sandbox base. // Note that the value returned by context.getId() is only unique inside one given SpawnRunner, // so we have to prefix our name to turn it into a globally unique value. Path sandboxPath = sandboxBase.getRelative(getName()).getRelative(Integer.toString(context.getId())); - sandboxPath.getParentDirectory().createDirectory(); - sandboxPath.createDirectory(); - // b/64689608: The execroot of the sandboxed process must end with the workspace name, just like - // the normal execroot does. - String workspaceName = execRoot.getBaseName(); - Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(workspaceName); - sandboxExecRoot.getParentDirectory().createDirectory(); - sandboxExecRoot.createDirectory(); + // The exec root base and the exec root of the sandbox from the point of view of the Bazel + // process (can be different from where the exec root appears within the sandbox due to file + // system namespace shenanigans). + Path sandboxExecRootBase = sandboxPath.getRelative("execroot"); + Path sandboxExecRoot = sandboxExecRootBase.getRelative(workspaceName); + // The directory that will be mounted as the hermetic /tmp, if any (otherwise null) Path sandboxTmp = null; - SandboxOptions sandboxOptions = getSandboxOptions(); - if (sandboxOptions.sandboxHermeticTmp) { - PathFragment tmpRoot = PathFragment.create("/tmp"); - // With a tmpfs on /tmp, mounting a disk-based hermetic /tmp isn't necessary. - if (!sandboxOptions.sandboxTmpfsPath.contains(tmpRoot)) { - // Mounting a tmpfs strictly below the hermetic /tmp isn't supported. We fall back to - // non-hermetic /tmp in that case, but print a warning mentioning the problematic mount. - Optional tmpfsPathUnderTmp = - sandboxOptions.sandboxTmpfsPath.stream() - .filter(path -> path.startsWith(tmpRoot)) - .findFirst(); - if (tmpfsPathUnderTmp.isEmpty()) { - sandboxTmp = sandboxPath.getRelative("_tmp"); - sandboxTmp.createDirectoryAndParents(); - } else 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()))); - } - } - } - - ImmutableMap environment = - localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp"); - ImmutableSet writableDirs = getWritableDirs(sandboxExecRoot, environment); + // These paths are paths that are visible for the processes running inside the sandbox. They + // can be different from paths from the point of view of the Bazel server because if we use + // hermetic /tmp and either the output base or a source root are under /tmp, they would be + // hidden by the newly mounted hermetic /tmp . So in that case, we make the sandboxed processes + // see the exec root, the source roots and the working directory of the action at constant + // locations under /tmp . + + // Base directory for source roots; each source root is a sequentially numbered subdirectory. + Path withinSandboxSourceRoots = null; + + // Working directory of the action; this is where the inputs (and only the inputs) of the action + // are visible. + Path withinSandboxWorkingDirectory = null; + + // The exec root. Necessary because the working directory contains symlinks to the execroot. + Path withinSandboxExecRoot = execRoot; + + boolean useHermeticTmp = useHermeticTmp(); + + if (useHermeticTmp) { + sandboxTmp = sandboxPath.getRelative("_tmp"); + withinSandboxSourceRoots = fileSystem.getPath(SLASH_TMP.getRelative(BAZEL_SOURCE_ROOTS)); + withinSandboxWorkingDirectory = + fileSystem + .getPath(SLASH_TMP.getRelative(BAZEL_WORKING_DIRECTORY)) + .getRelative(workspaceName); + withinSandboxExecRoot = + fileSystem.getPath(SLASH_TMP.getRelative(BAZEL_EXECROOT)).getRelative(workspaceName); + } SandboxInputs inputs = helpers.processInputFiles( context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot, - execRoot, + withinSandboxExecRoot, packageRoots, - null); - SandboxOutputs outputs = helpers.getOutputs(spawn); + withinSandboxSourceRoots); + + sandboxExecRoot.createDirectoryAndParents(); + + if (useHermeticTmp) { + for (Map.Entry root : inputs.getSourceRootBindMounts().entrySet()) { + createDirectoryWithinSandboxTmp(sandboxTmp, root.getKey().asPath()); + } + + createDirectoryWithinSandboxTmp(sandboxTmp, withinSandboxExecRoot); + createDirectoryWithinSandboxTmp(sandboxTmp, withinSandboxWorkingDirectory); + } + SandboxOutputs outputs = helpers.getOutputs(spawn); + ImmutableMap environment = + localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp"); + ImmutableSet writableDirs = + getWritableDirs( + sandboxExecRoot, useHermeticTmp ? withinSandboxExecRoot : sandboxExecRoot, environment); Duration timeout = context.getTimeout(); + SandboxOptions sandboxOptions = getSandboxOptions(); LinuxSandboxCommandLineBuilder commandLineBuilder = LinuxSandboxCommandLineBuilder.commandLineBuilder(linuxSandbox, spawn.getArguments()) .addExecutionInfo(spawn.getExecutionInfo()) .setWritableFilesAndDirectories(writableDirs) - .setTmpfsDirectories(ImmutableSet.copyOf(sandboxOptions.sandboxTmpfsPath)) - .setBindMounts(getBindMounts(blazeDirs, sandboxExecRoot, sandboxTmp)) - .setUseFakeHostname(sandboxOptions.sandboxFakeHostname) - .setEnablePseudoterminal(sandboxOptions.sandboxExplicitPseudoterminal) + .setTmpfsDirectories(ImmutableSet.copyOf(getSandboxOptions().sandboxTmpfsPath)) + .setBindMounts(getBindMounts(blazeDirs, inputs, sandboxExecRootBase, sandboxTmp)) + .setUseFakeHostname(getSandboxOptions().sandboxFakeHostname) + .setEnablePseudoterminal(getSandboxOptions().sandboxExplicitPseudoterminal) .setCreateNetworkNamespace( !(allowNetwork || Spawns.requiresNetwork(spawn, sandboxOptions.defaultSandboxAllowNetwork))) @@ -264,22 +347,23 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context UTF_8); } + if (useHermeticTmp) { + commandLineBuilder.setWorkingDirectory(withinSandboxWorkingDirectory); + } + if (!timeout.isZero()) { commandLineBuilder.setTimeout(timeout); } - if (spawn.getExecutionInfo().containsKey(ExecutionRequirements.REQUIRES_FAKEROOT)) { commandLineBuilder.setUseFakeRoot(true); } else if (sandboxOptions.sandboxFakeUsername) { commandLineBuilder.setUseFakeUsername(true); } - Path statisticsPath = null; if (sandboxOptions.collectLocalSandboxExecutionStatistics) { statisticsPath = sandboxPath.getRelative("stats.out"); commandLineBuilder.setStatisticsPath(statisticsPath); } - if (sandboxfsProcess != null) { return new SandboxfsSandboxedSpawn( sandboxfsProcess, @@ -329,10 +413,11 @@ public String getName() { } @Override - protected ImmutableSet getWritableDirs(Path sandboxExecRoot, Map env) + protected ImmutableSet getWritableDirs( + Path sandboxExecRoot, Path withinSandboxExecRoot, Map env) throws IOException { ImmutableSet.Builder writableDirs = ImmutableSet.builder(); - writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env)); + writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, withinSandboxExecRoot, env)); FileSystem fs = sandboxExecRoot.getFileSystem(); writableDirs.add(fs.getPath("/dev/shm").resolveSymbolicLinks()); @@ -341,41 +426,21 @@ protected ImmutableSet getWritableDirs(Path sandboxExecRoot, Map getBindMounts( - BlazeDirectories blazeDirs, Path sandboxExecRoot, @Nullable Path sandboxTmp) + private ImmutableList getBindMounts( + BlazeDirectories blazeDirs, + SandboxInputs inputs, + Path sandboxExecRootBase, + @Nullable Path sandboxTmp) throws UserExecException { Path tmpPath = fileSystem.getPath("/tmp"); final SortedMap bindMounts = Maps.newTreeMap(); - boolean buildUnderTmp = false; - if (blazeDirs.getWorkspace().startsWith(tmpPath)) { - bindMounts.put(blazeDirs.getWorkspace(), blazeDirs.getWorkspace()); - buildUnderTmp = true; - } - if (blazeDirs.getOutputBase().startsWith(tmpPath)) { - bindMounts.put(blazeDirs.getOutputBase(), blazeDirs.getOutputBase()); - buildUnderTmp = true; - } - if (sandboxTmp != null) { - if (buildUnderTmp) { - if (warnedAboutNonHermeticTmp.compareAndSet(false, true)) { - reporter.handle( - Event.warn( - "Falling back to non-hermetic /tmp in sandbox since workspace or output base " - + "lie under /tmp")); - } - } else { - // Mount a fresh, empty temporary directory as /tmp for each sandbox rather than reusing the - // host filesystem's /tmp. User-specified bind mounts can override this and use the host's - // /tmp instead by mounting /tmp to /tmp, if desired. - bindMounts.put(tmpPath, sandboxTmp); - } - } + for (ImmutableMap.Entry additionalMountPath : getSandboxOptions().sandboxAdditionalMounts) { try { final Path mountTarget = fileSystem.getPath(additionalMountPath.getValue()); // If source path is relative, treat it as a relative path inside the execution root - final Path mountSource = sandboxExecRoot.getRelative(additionalMountPath.getKey()); + final Path mountSource = sandboxExecRootBase.getRelative(additionalMountPath.getKey()); // If a target has more than one source path, the latter one will take effect. bindMounts.put(mountTarget, mountSource); } catch (IllegalArgumentException e) { @@ -385,6 +450,7 @@ private SortedMap getBindMounts( Code.BIND_MOUNT_ANALYSIS_FAILURE)); } } + for (Path inaccessiblePath : getInaccessiblePaths()) { if (inaccessiblePath.isDirectory(Symlinks.NOFOLLOW)) { bindMounts.put(inaccessiblePath, inaccessibleHelperDir); @@ -392,8 +458,35 @@ private SortedMap getBindMounts( bindMounts.put(inaccessiblePath, inaccessibleHelperFile); } } + validateBindMounts(bindMounts); - return bindMounts; + ImmutableList.Builder result = ImmutableList.builder(); + + if (sandboxTmp != null) { + // First mount the real exec root and the empty directory created as the working dir of the + // action under $SANDBOX/_tmp + result.add(BindMount.of(sandboxTmp.getRelative(BAZEL_EXECROOT), blazeDirs.getExecRootBase())); + result.add( + BindMount.of(sandboxTmp.getRelative(BAZEL_WORKING_DIRECTORY), sandboxExecRootBase)); + + // Then mount the individual package roots under $SANDBOX/_tmp/bazel-source-roots + for (Map.Entry sourceRoot : inputs.getSourceRootBindMounts().entrySet()) { + Path realSourceRoot = sourceRoot.getValue(); + Root withinSandboxSourceRoot = sourceRoot.getKey(); + PathFragment sandboxTmpSourceRoot = withinSandboxSourceRoot.asPath().relativeTo(tmpPath); + result.add(BindMount.of(sandboxTmp.getRelative(sandboxTmpSourceRoot), realSourceRoot)); + } + + // Then mount $SANDBOX/_tmp at /tmp. At this point, even if the output base (and execroot) + // and individual source roots are under /tmp, they are accessible at /tmp/bazel-* + result.add(BindMount.of(tmpPath, sandboxTmp)); + } + + for (Map.Entry bindMount : bindMounts.entrySet()) { + result.add(BindMount.of(bindMount.getKey(), bindMount.getValue())); + } + + return result.build(); } /** diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java index 21f8225daa5019..6d1aa9e65cc7e9 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java @@ -140,7 +140,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context environment, inputs, outputs, - getWritableDirs(sandboxExecRoot, environment), + getWritableDirs(sandboxExecRoot, sandboxExecRoot, environment), treeDeleter, statisticsPath, getSandboxOptions().reuseSandboxDirectories, diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java index b8ae7e363899a1..9f725b93068a91 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxHelpers.java @@ -369,6 +369,7 @@ public SandboxInputs( this.symlinks = symlinks; this.sourceRootBindMounts = sourceRootBindMounts; } + public static SandboxInputs getEmptyInputs() { return EMPTY_INPUTS; } @@ -381,6 +382,10 @@ public Map getSymlinks() { return symlinks; } + public Map getSourceRootBindMounts() { + return sourceRootBindMounts; + } + /** * Materializes a single virtual input inside the given execroot. * @@ -537,6 +542,24 @@ public SandboxInputs processInputFiles( if (actionInput instanceof EmptyActionInput) { inputPath = null; + } else if (actionInput instanceof Artifact) { + Artifact inputArtifact = (Artifact) actionInput; + if (inputArtifact.isSourceArtifact() && sandboxSourceRoots != null) { + Root sourceRoot = inputArtifact.getRoot().getRoot(); + if (!sourceRootToSandboxSourceRoot.containsKey(sourceRoot)) { + int next = sourceRootToSandboxSourceRoot.size(); + sourceRootToSandboxSourceRoot.put( + sourceRoot, + Root.fromPath(sandboxSourceRoots.getRelative(Integer.toString(next)))); + } + + inputPath = + RootedPath.toRootedPath( + sourceRootToSandboxSourceRoot.get(sourceRoot), + inputArtifact.getRootRelativePath()); + } else { + inputPath = RootedPath.toRootedPath(withinSandboxExecRoot, inputArtifact.getExecPath()); + } } else { PathFragment execPath = actionInput.getExecPath(); if (execPath.isAbsolute()) { diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java index b3b2faca588c36..5304367e293cd2 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/WindowsSandboxedSpawnRunner.java @@ -79,7 +79,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context null); ImmutableSet.Builder writablePaths = ImmutableSet.builder(); - writablePaths.addAll(getWritableDirs(execRoot, environment)); + writablePaths.addAll(getWritableDirs(execRoot, execRoot, environment)); for (ActionInput output : spawn.getOutputFiles()) { writablePaths.add(execRoot.getRelative(output.getExecPath())); } diff --git a/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java b/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java index 227253c01ea255..0cde9b2a9dcee5 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java +++ b/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java @@ -17,9 +17,9 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Maps; import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder; +import com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.BindMount; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; import com.google.devtools.build.lib.shell.Subprocess; @@ -30,7 +30,6 @@ import java.io.File; import java.io.IOException; import java.util.Set; -import java.util.SortedMap; import javax.annotation.Nullable; /** A {@link SingleplexWorker} that runs inside a sandboxed execution root. */ @@ -112,12 +111,13 @@ private ImmutableSet getWritableDirs(Path sandboxExecRoot) throws IOExcept return writableDirs.build(); } - private SortedMap getBindMounts(Path sandboxExecRoot, @Nullable Path sandboxTmp) { + private ImmutableList getBindMounts(Path sandboxExecRoot, @Nullable Path sandboxTmp) { Path tmpPath = sandboxExecRoot.getFileSystem().getPath("/tmp"); - final SortedMap bindMounts = Maps.newTreeMap(); + ImmutableList.Builder result = ImmutableList.builder(); // Mount a fresh, empty temporary directory as /tmp for each sandbox rather than reusing the // host filesystem's /tmp. Since we're in a worker, we clean this dir between requests. - bindMounts.put(tmpPath, sandboxTmp); + result.add(BindMount.of(tmpPath, sandboxTmp)); + return result.build(); // TODO(larsrc): Apply InaccessiblePaths // for (Path inaccessiblePath : getInaccessiblePaths()) { // if (inaccessiblePath.isDirectory(Symlinks.NOFOLLOW)) { @@ -127,7 +127,6 @@ private SortedMap getBindMounts(Path sandboxExecRoot, @Nullable Path // } // } // validateBindMounts(bindMounts); - return bindMounts; } @Override diff --git a/src/main/tools/linux-sandbox-pid1.cc b/src/main/tools/linux-sandbox-pid1.cc index 6c346168208cbc..7e13e0c64ff3e9 100644 --- a/src/main/tools/linux-sandbox-pid1.cc +++ b/src/main/tools/linux-sandbox-pid1.cc @@ -163,7 +163,7 @@ static int CreateTarget(const char *path, bool is_directory) { if (is_directory) { if (mkdir(path, 0755) < 0) { - DIE("mkdir"); + DIE("mkdir(%s)", path); } } else { LinkFile(path); @@ -297,12 +297,6 @@ static void MountFilesystems() { } } - if (mount(opt.working_dir.c_str(), opt.working_dir.c_str(), nullptr, MS_BIND, - nullptr) < 0) { - DIE("mount(%s, %s, nullptr, MS_BIND, nullptr)", opt.working_dir.c_str(), - opt.working_dir.c_str()); - } - std::unordered_set bind_mount_sources; for (size_t i = 0; i < opt.bind_mount_sources.size(); i++) { @@ -310,8 +304,9 @@ static void MountFilesystems() { bind_mount_sources.insert(source); const std::string &target = opt.bind_mount_targets.at(i); PRINT_DEBUG("bind mount: %s -> %s", source.c_str(), target.c_str()); - if (mount(source.c_str(), target.c_str(), nullptr, MS_BIND, nullptr) < 0) { - DIE("mount(%s, %s, nullptr, MS_BIND, nullptr)", source.c_str(), + if (mount(source.c_str(), target.c_str(), nullptr, MS_BIND | MS_REC, + nullptr) < 0) { + DIE("mount(%s, %s, nullptr, MS_BIND | MS_REC, nullptr)", source.c_str(), target.c_str()); } } @@ -330,6 +325,12 @@ static void MountFilesystems() { writable_file.c_str(), writable_file.c_str()); } } + + if (mount(opt.working_dir.c_str(), opt.working_dir.c_str(), nullptr, MS_BIND, + nullptr) < 0) { + DIE("mount(%s, %s, nullptr, MS_BIND, nullptr)", opt.working_dir.c_str(), + opt.working_dir.c_str()); + } } // We later remount everything read-only, except the paths for which this method diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java index 2eea4118aa081d..55d278ea0d26f9 100644 --- a/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java +++ b/src/test/java/com/google/devtools/build/lib/sandbox/LinuxSandboxCommandLineBuilderTest.java @@ -19,7 +19,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedMap; +import com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.BindMount; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; @@ -123,12 +123,11 @@ public void testLinuxSandboxCommandLineBuilder_buildsWithOptionalArguments() { ImmutableSet tmpfsDirectories = ImmutableSet.of(tmpfsDir1, tmpfsDir2); - ImmutableSortedMap bindMounts = - ImmutableSortedMap.naturalOrder() - .put(bindMountTarget1, bindMountSource1) - .put(bindMountTarget2, bindMountSource2) - .put(bindMountSameSourceAndTarget, bindMountSameSourceAndTarget) - .buildOrThrow(); + ImmutableList bindMounts = + ImmutableList.of( + BindMount.of(bindMountSameSourceAndTarget, bindMountSameSourceAndTarget), + BindMount.of(bindMountTarget1, bindMountSource1), + BindMount.of(bindMountTarget2, bindMountSource2)); String cgroupsDir = "/sys/fs/cgroups/something"; diff --git a/src/test/shell/bazel/bazel_sandboxing_test.sh b/src/test/shell/bazel/bazel_sandboxing_test.sh index 6200a062e4c674..46a5417f2a3abf 100755 --- a/src/test/shell/bazel/bazel_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_sandboxing_test.sh @@ -909,6 +909,100 @@ EOF --test_output=errors &>$TEST_log || fail "Expected test to pass" } +function test_hermetic_tmp_under_tmp { + if [[ "$(uname -s)" != Linux ]]; then + echo "Skipping test: --incompatible_sandbox_hermetic_tmp is only supported in Linux" 1>&2 + return 0 + fi + + temp_dir=$(mktemp -d /tmp/test.XXXXXX) + trap 'rm -rf ${temp_dir}' EXIT + + mkdir -p "${temp_dir}/workspace/a" + mkdir -p "${temp_dir}/package-path/b" + mkdir -p "${temp_dir}/repo/c" + mkdir -p "${temp_dir}/output-base" + + cd "${temp_dir}/workspace" + cat > WORKSPACE < a/BUILD <<'EOF' +genrule( + name = "g", + outs = ["go"], + srcs = [], + cmd = "echo GO > $@", +) + +sh_binary( + name = "bin", + srcs = ["bin.sh"], + data = [":s", ":go", "//b:s", "//b:go", "@repo//c:s", "@repo//c:go"], +) + +genrule( + name = "t", + tools = [":bin"], + srcs = [":s", ":go", "//b:s", "//b:go", "@repo//c:s", "@repo//c:go"], + outs = ["to"], + cmd = "\n".join([ + "RUNFILES=$(location :bin).runfiles/__main__", + "S=$(location :s); GO=$(location :go)", + "BS=$(location //b:s); BGO=$(location //b:go)", + "RS=$(location @repo//c:s); RGO=$(location @repo//c:go)", + "for i in $$S $$GO $$BS $$BGO $$RS $$RGO; do", + " echo reading $$i", + " cat $$i >> $@", + "done", + "for i in a/s a/go b/s b/go ../repo/c/s ../repo/c/go; do", + " echo reading $$RUNFILES/$$i", + " cat $$RUNFILES/$$i >> $@", + "done", + ]), +) +EOF + + touch a/bin.sh + chmod +x a/bin.sh + + touch ../repo/WORKSPACE + cat > ../repo/c/BUILD <<'EOF' +exports_files(["s"]) + +genrule( + name = "g", + outs = ["go"], + srcs = [], + cmd = "echo GO > $@", + visibility = ["//visibility:public"], +) +EOF + + cat > ../package-path/b/BUILD <<'EOF' +exports_files(["s"]) + +genrule( + name = "g", + outs = ["go"], + srcs = [], + cmd = "echo GO > $@", + visibility = ["//visibility:public"], +) +EOF + + touch "a/s" "../package-path/b/s" "../repo/c/s" + + cat WORKSPACE + bazel \ + --output_base="${temp_dir}/output-base" \ + build \ + --incompatible_sandbox_hermetic_tmp \ + --package_path="%workspace%:${temp_dir}/package-path" \ + //a:t || fail "build failed" +} + function test_read_hermetic_tmp { if [[ "$(uname -s)" != Linux ]]; then echo "Skipping test: --incompatible_sandbox_hermetic_tmp is only supported in Linux" 1>&2