From 4b40a5fccd3c9c63eea44b3eb173760dc88be743 Mon Sep 17 00:00:00 2001 From: Philipp Wollermann Date: Tue, 6 Oct 2015 11:49:28 +0000 Subject: sandbox: Improve MountMap to always return entries sorted by path depth and lexicographical order. This prevents certain edge cases in the sandbox, where a mounted child directory could be hidden by a later mount of a parent directory over its parent. -- MOS_MIGRATED_REVID=104749937 --- .../build/lib/sandbox/LinuxSandboxedStrategy.java | 43 ++++++++++------------ .../devtools/build/lib/sandbox/MountMap.java | 22 ++++++----- 2 files changed, 32 insertions(+), 33 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/sandbox') diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java index 945f267f03..a7657ad6af 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java @@ -193,7 +193,7 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { private ImmutableMap getMounts( Spawn spawn, ActionExecutionContext executionContext) throws IOException { - MountMap mounts = new MountMap<>(); + MountMap mounts = new MountMap(); mounts.putAll(mountUsualUnixDirs()); mounts.putAll(withRecursedDirs(setupBlazeUtils())); mounts.putAll(withRecursedDirs(mountRunfilesFromManifests(spawn))); @@ -230,9 +230,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { * @return a new mounts multimap with the added mounts. */ @VisibleForTesting - static MountMap withResolvedSymlinks(Map mounts) - throws IOException { - MountMap fixedMounts = new MountMap<>(); + static MountMap withResolvedSymlinks(Map mounts) throws IOException { + MountMap fixedMounts = new MountMap(); for (Entry mount : mounts.entrySet()) { Path target = mount.getKey(); Path source = mount.getValue(); @@ -253,8 +252,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { * @return a new mounts multimap with the added mounts. */ @VisibleForTesting - static MountMap withRecursedDirs(Map mounts) throws IOException { - MountMap fixedMounts = new MountMap<>(); + static MountMap withRecursedDirs(Map mounts) throws IOException { + MountMap fixedMounts = new MountMap(); for (Entry mount : mounts.entrySet()) { Path target = mount.getKey(); Path source = mount.getValue(); @@ -275,8 +274,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { * Mount a certain set of unix directories to make the usual tools and libraries available to the * spawn that runs. */ - private MountMap mountUsualUnixDirs() throws IOException { - MountMap mounts = new MountMap<>(); + private MountMap mountUsualUnixDirs() throws IOException { + MountMap mounts = new MountMap(); FileSystem fs = blazeDirs.getFileSystem(); mounts.put(fs.getPath("/bin"), fs.getPath("/bin")); mounts.put(fs.getPath("/etc"), fs.getPath("/etc")); @@ -298,8 +297,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { /** * Mount the embedded tools. */ - private MountMap setupBlazeUtils() throws IOException { - MountMap mounts = new MountMap<>(); + private MountMap setupBlazeUtils() throws IOException { + MountMap mounts = new MountMap(); Path mount = blazeDirs.getEmbeddedBinariesRoot().getRelative("build-runfiles"); mounts.put(mount, mount); return mounts; @@ -308,9 +307,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { /** * Mount all runfiles that the spawn needs as specified in its runfiles manifests. */ - private MountMap mountRunfilesFromManifests(Spawn spawn) - throws IOException { - MountMap mounts = new MountMap<>(); + private MountMap mountRunfilesFromManifests(Spawn spawn) throws IOException { + MountMap mounts = new MountMap(); for (Entry manifest : spawn.getRunfilesManifests().entrySet()) { String manifestFilePath = manifest.getValue().getPath().getPathString(); Preconditions.checkState(!manifest.getKey().isAbsolute()); @@ -321,9 +319,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { return mounts; } - static MountMap parseManifestFile( - Path targetDirectory, File manifestFile) throws IOException { - MountMap mounts = new MountMap<>(); + static MountMap parseManifestFile(Path targetDirectory, File manifestFile) throws IOException { + MountMap mounts = new MountMap(); for (String line : Files.readLines(manifestFile, Charset.defaultCharset())) { String[] fields = line.trim().split(" "); Path source; @@ -346,9 +343,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { /** * Mount all runfiles that the spawn needs as specified via its runfiles suppliers. */ - private MountMap mountRunfilesFromSuppliers(Spawn spawn) - throws IOException { - MountMap mounts = new MountMap<>(); + private MountMap mountRunfilesFromSuppliers(Spawn spawn) throws IOException { + MountMap mounts = new MountMap(); FileSystem fs = blazeDirs.getFileSystem(); Map> rootsAndMappings = spawn.getRunfilesSupplier().getMappings(); @@ -370,10 +366,9 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { /** * Mount all inputs of the spawn. */ - private MountMap mountInputs( - Spawn spawn, ActionExecutionContext actionExecutionContext) + private MountMap mountInputs(Spawn spawn, ActionExecutionContext actionExecutionContext) throws IOException { - MountMap mounts = new MountMap<>(); + MountMap mounts = new MountMap(); List inputs = ActionInputHelper.expandMiddlemen( @@ -404,8 +399,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext { *

If --run_under= refers to a label, it is automatically provided in the spawn's input files, * so mountInputs() will catch that case. */ - private MountMap mountRunUnderCommand(Spawn spawn) { - MountMap mounts = new MountMap<>(); + private MountMap mountRunUnderCommand(Spawn spawn) { + MountMap mounts = new MountMap(); if (spawn.getResourceOwner() instanceof TestRunnerAction) { TestRunnerAction testRunnerAction = ((TestRunnerAction) spawn.getResourceOwner()); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/MountMap.java b/src/main/java/com/google/devtools/build/lib/sandbox/MountMap.java index 4e0ae07e41..5051d27df3 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/MountMap.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/MountMap.java @@ -13,26 +13,30 @@ // limitations under the License. package com.google.devtools.build.lib.sandbox; -import com.google.common.collect.ForwardingMap; +import com.google.common.collect.ForwardingSortedMap; +import com.google.devtools.build.lib.vfs.Path; -import java.util.LinkedHashMap; import java.util.Map; +import java.util.SortedMap; +import java.util.TreeMap; /** * A map that throws an exception when trying to replace a key (i.e. once a key gets a value, * any additional attempt of putting a value on the same key will throw an exception). + * + *

Returns entries sorted by path depth (shorter paths first) and in lexicographical order. */ -final class MountMap extends ForwardingMap { - final LinkedHashMap delegate = new LinkedHashMap<>(); +final class MountMap extends ForwardingSortedMap { + final TreeMap delegate = new TreeMap<>(); @Override - protected Map delegate() { + protected SortedMap delegate() { return delegate; } @Override - public V put(K key, V value) { - V previousValue = get(key); + public Path put(Path key, Path value) { + Path previousValue = get(key); if (previousValue == null) { return super.put(key, value); } else if (previousValue.equals(value)) { @@ -44,8 +48,8 @@ final class MountMap extends ForwardingMap { } @Override - public void putAll(Map map) { - for (Entry entry : map.entrySet()) { + public void putAll(Map map) { + for (Entry entry : map.entrySet()) { put(entry.getKey(), entry.getValue()); } } -- cgit v1.2.3