From 8e32f44a279c5c4e9c24fb84a08276ffe4fe90c0 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 12 Dec 2022 00:40:58 -0800 Subject: [PATCH] This change makes it possible to use the Linux sandbox when either the execroot, some package path entries, or both are under /tmp. This is achieved by a reshuffling of the sandbox directory layout in the following way: * The exec root base is `/tmp/bazel-working-directory` (cwd is under it) * Source roots are mapped under `/tmp/bazel-source-roots/$NUMBER` * The "real" exec root is mapped to `/tmp/bazel-execroot`. * All this is achieved with subtle manipulation of bind mounts: 1. The real exec root (bazel info execution_root) under `$SANDBOX/_tmp/bazel-execroot` 2. The sandbox exec root (the symlink tree the sandbox creates) under `$SANDBOX/_tmp/bazel-working-directory` 3. Each source root under `$SANDBOX/_tmp/bazel-source-roots/$NUMBER` 4. `$SANDBOX/_tmp` under `/tmp` This makes the directories in (1), (2) and (3) available as `/tmp/$NAME` even if they were originally under `/tmp` (which gets clobbered in step (4)) The functionality is gated under `--incompatible_sandbox_hermetic_tmp` since it requires `/tmp` to be in a known state, which only that flag can guarantee. Notably, putting these three directories under `/` does not work, because the non-hermetic sandbox uses the real file system and the root directory is not writable. We could conceivably get around that by bind mounting every first child of the "real" root directory in the sandbox root directory and using a writable directory as the sandbox root, but why bother if this one works. Progress towards #3236 (the flag still needs to be flipped) RELNOTES: None. PiperOrigin-RevId: 494650851 Change-Id: I0b3d1baf748a357a5bb4dee799cf807c2d75ef15 --- .../sandbox/AbstractSandboxSpawnRunner.java | 8 +- .../google/devtools/build/lib/sandbox/BUILD | 1 + .../sandbox/DarwinSandboxedSpawnRunner.java | 3 +- .../LinuxSandboxCommandLineBuilder.java | 29 ++- .../sandbox/LinuxSandboxedSpawnRunner.java | 243 ++++++++++++------ .../ProcessWrapperSandboxedSpawnRunner.java | 2 +- .../build/lib/sandbox/SandboxHelpers.java | 23 ++ .../sandbox/WindowsSandboxedSpawnRunner.java | 2 +- .../build/lib/worker/SandboxedWorker.java | 11 +- src/main/tools/linux-sandbox-pid1.cc | 19 +- .../LinuxSandboxCommandLineBuilderTest.java | 13 +- src/test/shell/bazel/bazel_sandboxing_test.sh | 94 +++++++ 12 files changed, 338 insertions(+), 110 deletions(-) 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