Skip to content

Commit

Permalink
Simplify sandbox setup and fix integration test
Browse files Browse the repository at this point in the history
  • Loading branch information
@fmeum
fmeum committed May 14, 2024
1 parent 612da61 commit 9c0c738
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 90 deletions.
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/sandbox/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:caffeine",
"//third_party:guava",
"//third_party:jsr305",
],
Expand Down
148 changes: 59 additions & 89 deletions src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS_WITH_LOOPBACK;
import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NO_NETNS;
import static java.util.stream.Collectors.joining;

import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.LoadingCache;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -62,15 +58,13 @@
import java.io.IOException;
import java.time.Duration;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.SequencedSet;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.concurrent.CompletionException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Stream;
import javax.annotation.Nullable;

/** Spawn runner that uses linux sandboxing APIs to execute a local subprocess. */
Expand Down Expand Up @@ -138,10 +132,7 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS
private final TreeDeleter treeDeleter;
private final Reporter reporter;
private final Path slashTmp;
private final Path outputBase;
private final ImmutableList<Root> packagePath;
private final LoadingCache<Root, ImmutableSet<Path>> symlinkChainCache =
Caffeine.newBuilder().build(LinuxSandboxedSpawnRunner::readSymlinkChainUncached);
private final ImmutableSet<Path> knownPathsToMountUnderHermeticTmp;
private String cgroupsDir;

/**
Expand Down Expand Up @@ -176,8 +167,41 @@ private static boolean computeIsSupported(CommandEnvironment cmdEnv, Path linuxS
this.treeDeleter = treeDeleter;
this.reporter = cmdEnv.getReporter();
this.slashTmp = cmdEnv.getRuntime().getFileSystem().getPath("/tmp");
this.outputBase = cmdEnv.getOutputBase();
this.packagePath = cmdEnv.getPackageLocator().getPathEntries();
this.knownPathsToMountUnderHermeticTmp = collectPathsToMountUnderHermeticTmp(cmdEnv);
}

private ImmutableSet<Path> collectPathsToMountUnderHermeticTmp(CommandEnvironment cmdEnv) {
// If any path managed or tracked by Bazel is under /tmp, it needs to be explicitly mounted
// into the sandbox when using hermetic /tmp. We attempt to collect an over-approximation of
// these paths, as the main goal of hermetic /tmp is to avoid inheriting any direct
// or well-known children of /tmp from the host.
return Stream.concat(
Stream.of(cmdEnv.getOutputBase()),
cmdEnv.getPackageLocator().getPathEntries().stream().map(Root::asPath))
.filter(p -> p.startsWith(slashTmp))
// For any path /tmp/dir1/dir2 we encounter, we instead mount /tmp/dir1 (first two
// path segments). This is necessary to gracefully handle an edge case:
// - A workspace contains a subdirectory (e.g. examples) that is itself a workspace.
// - The child workspace brings in the parent workspace as a local_repository with
// an up-level reference.
// - The parent workspace is checked out under /tmp.
// In this scenario, the parent workspace's external source root points to the parent
// workspace's source directory under /tmp, but this directory is neither under the
// output base nor on the package path. While it would be possible to track the
// external roots of all inputs and mount their entire symlink chain, this would be
// very invasive to do in the face of resolved symlink artifacts (and impossible with
// unresolved symlinks).
// Instead, by mounting the direct children of /tmp that are parents of the source
// roots, we attempt to cover all reasonable cases in which repositories symlink
// paths relative to themselves and workspaces are checked out into subdirectories of
// /tmp. All explicit references to paths under /tmp must be handled by the user via
// --sandbox_add_mount_pair.
.map(
p ->
p.getFileSystem()
.getPath(
p.asFragment().subFragment(0, Math.min(2, p.asFragment().segmentCount()))))
.collect(toImmutableSet());
}

private boolean useHermeticTmp() {
Expand All @@ -200,6 +224,13 @@ private boolean useHermeticTmp() {
return false;
}

if (knownPathsToMountUnderHermeticTmp.contains(slashTmp)) {
// /tmp as a package path entry or output base seems very unlikely to work, but the bind
// mounting logic is not prepared for it and we don't want to crash, so just disable hermetic
// tmp in this case.
return false;
}

if (getSandboxOptions().sandboxTmpfsPath.contains(slashTmp.asFragment())) {
// A tmpfs path under /tmp is as hermetic as "hermetic /tmp".
return false;
Expand Down Expand Up @@ -232,50 +263,21 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
Path sandboxTmp = null;
ImmutableSet<Path> pathsUnderTmpToMount = ImmutableSet.of();
if (useHermeticTmp()) {
// Roots under /tmp are treated exactly like a user mount under /tmp to ensure that they are
// visible at the same path after mounting the hermetic tmp. Since a source root can be a
// symlink to a directory under /tmp, we need to account for all intermediate symlinks.
SequencedSet<Path> resolvedRoots = new LinkedHashSet<>();
resolvedRoots.add(outputBase);
for (Root root : Iterables.concat(packagePath)) {
if (!root.asPath().startsWith(outputBase)) {
resolvedRoots.add(root.asPath());
}
// resolvedRoots.addAll(readSymlinkChain(root));
}
// /tmp as a package path entry, output base or target of a local_repository seems very
// unlikely to work, but the bind mounting logic is not prepared for it and we don't want to
// crash, so just disable hermetic tmp in this case.
if (!resolvedRoots.contains(slashTmp)) {
pathsUnderTmpToMount =
resolvedRoots.stream()
.filter(p -> p.startsWith(slashTmp))
// For any path /tmp/dir1/dir2 we encounter, we instead mount /tmp/dir1 (first two
// path segments). This is necessary to gracefully handle an edge case:
// - A workspace contains a subdirectory (e.g. examples) that is itself a workspace.
// - The child workspace brings in the parent workspace as a local_repository with
// an up-level reference.
// - The parent workspace is checked out under /tmp.
// - The parent workspace uses resolved symlink artifacts.
// In this scenario, the symlinks point to the parent workspace's external source
// root, which in turn points to the parent workspace's source directory under /tmp.
// Since the symlinks are not followed when finding the resolved roots above, only
// the child workspace's directory shows up in resolvedRoots.
.map(p -> p.getFileSystem().getPath(p.asFragment().subFragment(0, 2)))
.collect(toImmutableSet());

// The initially empty directory that will be mounted as /tmp in the sandbox.
sandboxTmp = sandboxPath.getRelative("_hermetic_tmp");
sandboxTmp.createDirectoryAndParents();

for (PathFragment pathFragment : getSandboxOptions().sandboxTmpfsPath) {
Path path = fileSystem.getPath(pathFragment);
if (path.startsWith(slashTmp)) {
// 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.
sandboxTmp.getRelative(path.relativeTo(slashTmp)).createDirectoryAndParents();
}
// Special paths under /tmp are treated exactly like a user mount under /tmp to ensure that
// they are visible at the same path after mounting the hermetic tmp.
pathsUnderTmpToMount = knownPathsToMountUnderHermeticTmp;

// The initially empty directory that will be mounted as /tmp in the sandbox.
sandboxTmp = sandboxPath.getRelative("_hermetic_tmp");
sandboxTmp.createDirectoryAndParents();

for (PathFragment pathFragment : getSandboxOptions().sandboxTmpfsPath) {
Path path = fileSystem.getPath(pathFragment);
if (path.startsWith(slashTmp)) {
// 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.
sandboxTmp.getRelative(path.relativeTo(slashTmp)).createDirectoryAndParents();
}
}
}
Expand Down Expand Up @@ -426,38 +428,6 @@ private ImmutableMap<Path, Path> prepareAndGetBindMounts(
.buildOrThrow();
}

/** Returns the chain of symlink targets obtained while resolving the path of the given root. */
private ImmutableSet<Path> readSymlinkChain(Root root) throws IOException {
// We don't expect roots to change where they point to during a single build, so we can cache
// the symlink chains. The root path itself is not stored to reduce memory usage by having most
// roots share the empty set instance.
try {
return symlinkChainCache.get(root);
} catch (CompletionException e) {
Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
throw e;
}
}

private static ImmutableSet<Path> readSymlinkChainUncached(Root root) throws IOException {
Path path = root.asPath();
SequencedSet<Path> result = new LinkedHashSet<>();
while (true) {
try {
path = path.getParentDirectory().getRelative(path.readSymbolicLink());
} catch (FileSystem.NotASymlinkException e) {
break;
}
if (!result.add(path)) {
throw new IOException(
String.format(
"Cycle in symlink chain for root %s: %s",
root, result.stream().map(Path::toString).collect(joining(" -> "))));
}
}
return ImmutableSet.copyOf(result);
}

@Override
public void verifyPostCondition(
Spawn originalSpawn, SandboxedSpawn sandbox, SpawnExecutionContext context)
Expand Down

0 comments on commit 9c0c738

Please sign in to comment.