sandbox: No longer require a temporary "sandbox" directory.
By removing the --sandbox_block_path feature in an earlier change and
taking advantage of the fact that in a mount namespace we can actually
"remount" mount points to be read-only without bind mounting them to
some other place beforehand, this is no longer necessary. The code
becomes much simpler due to this, for example we no longer need to
chroot.
--
PiperOrigin-RevId: 151111360
MOS_MIGRATED_REVID=151111360
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
index 122bd76..29d9084 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxRunner.java
@@ -40,7 +40,6 @@
private final Path execRoot;
private final Path sandboxExecRoot;
- private final Path sandboxTempDir;
private final Path argumentsFilePath;
private final Set<Path> writableDirs;
private final Set<Path> tmpfsPaths;
@@ -52,7 +51,6 @@
Path execRoot,
Path sandboxPath,
Path sandboxExecRoot,
- Path sandboxTempDir,
Set<Path> writableDirs,
Set<Path> tmpfsPaths,
Map<Path, Path> bindMounts,
@@ -61,7 +59,6 @@
super(verboseFailures);
this.execRoot = execRoot;
this.sandboxExecRoot = sandboxExecRoot;
- this.sandboxTempDir = sandboxTempDir;
this.argumentsFilePath = sandboxPath.getRelative("linux-sandbox.params");
this.writableDirs = writableDirs;
this.tmpfsPaths = tmpfsPaths;
@@ -128,10 +125,6 @@
fileArgs.add("-D");
}
- // Temporary directory of the sandbox.
- fileArgs.add("-S");
- fileArgs.add(sandboxTempDir.toString());
-
// Working directory of the spawn.
fileArgs.add("-W");
fileArgs.add(sandboxExecRoot.toString());
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 7ab7900..cc7da50 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
@@ -129,7 +129,6 @@
// Each invocation of "exec" gets its own sandbox.
Path sandboxPath = SandboxHelpers.getSandboxRoot(blazeDirs, productName, uuid, execCounter);
Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName());
- Path sandboxTempDir = sandboxPath.getRelative("tmp");
Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment());
@@ -138,12 +137,11 @@
try {
symlinkedExecRoot.createFileSystem(
getMounts(spawn, actionExecutionContext), outputs, writableDirs);
- sandboxTempDir.createDirectory();
} catch (IOException e) {
throw new UserExecException("I/O error during sandboxed execution", e);
}
- SandboxRunner runner = getSandboxRunner(spawn, sandboxPath, sandboxExecRoot, sandboxTempDir);
+ SandboxRunner runner = getSandboxRunner(spawn, sandboxPath, sandboxExecRoot);
try {
runSpawn(
spawn,
@@ -170,15 +168,13 @@
}
}
- private SandboxRunner getSandboxRunner(
- Spawn spawn, Path sandboxPath, Path sandboxExecRoot, Path sandboxTempDir)
+ private SandboxRunner getSandboxRunner(Spawn spawn, Path sandboxPath, Path sandboxExecRoot)
throws UserExecException {
if (fullySupported) {
return new LinuxSandboxRunner(
execRoot,
sandboxPath,
sandboxExecRoot,
- sandboxTempDir,
getWritableDirs(sandboxExecRoot, spawn.getEnvironment()),
getTmpfsPaths(),
getReadOnlyBindMounts(blazeDirs, sandboxExecRoot),
diff --git a/src/main/tools/linux-sandbox-options.cc b/src/main/tools/linux-sandbox-options.cc
index 3bd9a7e..96354ec 100644
--- a/src/main/tools/linux-sandbox-options.cc
+++ b/src/main/tools/linux-sandbox-options.cc
@@ -123,23 +123,14 @@
int c;
bool source_specified;
- while ((c = getopt(args->size(), args->data(),
- ":CS:W:T:t:l:L:w:e:M:m:HNRD")) != -1) {
+ while ((c = getopt(args->size(), args->data(), ":CW:T:t:l:L:w:e:M:m:HNRD")) !=
+ -1) {
if (c != 'M' && c != 'm') source_specified = false;
switch (c) {
case 'C':
// Shortcut for the "does this system support sandboxing" check.
exit(CheckNamespacesSupported());
break;
- case 'S':
- if (opt.sandbox_root_dir == NULL) {
- ValidateIsAbsolutePath(optarg, args->front(), static_cast<char>(c));
- opt.sandbox_root_dir = strdup(optarg);
- } else {
- Usage(args->front(),
- "Multiple root directories (-S) specified, expected one.");
- }
- break;
case 'W':
if (opt.working_dir == NULL) {
ValidateIsAbsolutePath(optarg, args->front(), static_cast<char>(c));
diff --git a/src/main/tools/linux-sandbox-options.h b/src/main/tools/linux-sandbox-options.h
index 6f57eba..5c78e46 100644
--- a/src/main/tools/linux-sandbox-options.h
+++ b/src/main/tools/linux-sandbox-options.h
@@ -22,8 +22,6 @@
// Options parsing result.
struct Options {
- // Temporary root directory (-S)
- const char *sandbox_root_dir;
// Working directory (-W)
const char *working_dir;
// How long to wait before killing the child (-T)
diff --git a/src/main/tools/linux-sandbox-pid1.cc b/src/main/tools/linux-sandbox-pid1.cc
index 0be62d9..76dccba 100644
--- a/src/main/tools/linux-sandbox-pid1.cc
+++ b/src/main/tools/linux-sandbox-pid1.cc
@@ -145,102 +145,37 @@
}
}
-// Recursively creates the file or directory specified in "path" and its parent
-// directories.
-static int CreateTarget(const char *path, bool is_directory) {
- PRINT_DEBUG("CreateTarget(%s, %s)", path, is_directory ? "true" : "false");
- if (path == NULL) {
- errno = EINVAL;
- return -1;
- }
-
- struct stat sb;
- // If the path already exists...
- if (stat(path, &sb) == 0) {
- if (is_directory && S_ISDIR(sb.st_mode)) {
- // and it's a directory and supposed to be a directory, we're done here.
- return 0;
- } else if (!is_directory && S_ISREG(sb.st_mode)) {
- // and it's a regular file and supposed to be one, we're done here.
- return 0;
- } else {
- // otherwise something is really wrong.
- errno = is_directory ? ENOTDIR : EEXIST;
- return -1;
- }
- } else {
- // If stat failed because of any error other than "the path does not exist",
- // this is an error.
- if (errno != ENOENT) {
- return -1;
- }
- }
-
- // Create the parent directory.
- if (CreateTarget(dirname(strdupa(path)), true) < 0) {
- DIE("CreateTarget(%s, true)", dirname(strdupa(path)));
- }
-
- if (is_directory) {
- if (mkdir(path, 0755) < 0) {
- DIE("mkdir(%s, 0755)", path);
- }
- } else {
- int handle;
- if ((handle = open(path, O_CREAT | O_WRONLY | O_EXCL, 0666)) < 0) {
- DIE("open(%s, O_CREAT | O_WRONLY | O_EXCL, 0666)", path);
- }
- if (close(handle) < 0) {
- DIE("close(%d)", handle);
- }
- }
-
- return 0;
-}
-
static void MountFilesystems() {
- if (mount("/", opt.sandbox_root_dir, NULL, MS_BIND | MS_REC, NULL) < 0) {
- DIE("mount(/, %s, NULL, MS_BIND | MS_REC, NULL)", opt.sandbox_root_dir);
- }
-
- if (chdir(opt.sandbox_root_dir) < 0) {
- DIE("chdir(%s)", opt.sandbox_root_dir);
- }
-
for (const char *tmpfs_dir : opt.tmpfs_dirs) {
PRINT_DEBUG("tmpfs: %s", tmpfs_dir);
- if (mount("tmpfs", tmpfs_dir + 1, "tmpfs",
- MS_NOSUID | MS_NODEV | MS_NOATIME, NULL) < 0) {
+ if (mount("tmpfs", tmpfs_dir, "tmpfs", MS_NOSUID | MS_NODEV | MS_NOATIME,
+ NULL) < 0) {
DIE("mount(tmpfs, %s, tmpfs, MS_NOSUID | MS_NODEV | MS_NOATIME, NULL)",
- tmpfs_dir + 1);
+ tmpfs_dir);
}
}
// Make sure that our working directory is a mount point. The easiest way to
// do this is by bind-mounting it upon itself.
PRINT_DEBUG("working dir: %s", opt.working_dir);
- PRINT_DEBUG("sandbox root: %s", opt.sandbox_root_dir);
- CreateTarget(opt.working_dir + 1, true);
- if (mount(opt.working_dir, opt.working_dir + 1, NULL, MS_BIND, NULL) < 0) {
- DIE("mount(%s, %s, NULL, MS_BIND, NULL)", opt.working_dir,
- opt.working_dir + 1);
+ if (mount(opt.working_dir, opt.working_dir, NULL, MS_BIND, NULL) < 0) {
+ DIE("mount(%s, %s, NULL, MS_BIND, NULL)", opt.working_dir, opt.working_dir);
}
for (size_t i = 0; i < opt.bind_mount_sources.size(); i++) {
const char *source = opt.bind_mount_sources.at(i);
const char *target = opt.bind_mount_targets.at(i);
PRINT_DEBUG("bind mount: %s -> %s", source, target);
- if (mount(source, target + 1, NULL, MS_BIND, NULL) < 0) {
- DIE("mount(%s, %s, NULL, MS_BIND, NULL)", source, target + 1);
+ if (mount(source, target, NULL, MS_BIND, NULL) < 0) {
+ DIE("mount(%s, %s, NULL, MS_BIND, NULL)", source, target);
}
}
for (const char *writable_file : opt.writable_files) {
PRINT_DEBUG("writable: %s", writable_file);
- if (mount(writable_file, writable_file + 1, NULL, MS_BIND, NULL) < 0) {
- DIE("mount(%s, %s, NULL, MS_BIND, NULL)", writable_file,
- writable_file + 1);
+ if (mount(writable_file, writable_file, NULL, MS_BIND, NULL) < 0) {
+ DIE("mount(%s, %s, NULL, MS_BIND, NULL)", writable_file, writable_file);
}
}
}
@@ -248,8 +183,6 @@
// We later remount everything read-only, except the paths for which this method
// returns true.
static bool ShouldBeWritable(char *mnt_dir) {
- mnt_dir += strlen(opt.sandbox_root_dir);
-
if (strcmp(mnt_dir, opt.working_dir) == 0) {
return true;
}
@@ -288,14 +221,10 @@
struct mntent *ent;
while ((ent = getmntent(mounts)) != NULL) {
- // Skip mounts that do not belong to our sandbox.
- if (strstr(ent->mnt_dir, opt.sandbox_root_dir) != ent->mnt_dir) {
- continue;
- }
// Skip mounts that are under tmpfs directories because we've already
// replaced such directories with new tmpfs instances.
// mount() would fail with ENOENT if we tried to remount such mount points.
- if (IsUnderTmpDir(ent->mnt_dir + strlen(opt.sandbox_root_dir))) {
+ if (IsUnderTmpDir(ent->mnt_dir)) {
continue;
}
@@ -355,7 +284,7 @@
static void MountProc() {
// Mount a new proc on top of the old one, because the old one still refers to
// our parent PID namespace.
- if (mount("proc", "proc", "proc", MS_NODEV | MS_NOEXEC | MS_NOSUID, NULL) <
+ if (mount("/proc", "/proc", "proc", MS_NODEV | MS_NOEXEC | MS_NOSUID, NULL) <
0) {
DIE("mount");
}
@@ -393,29 +322,6 @@
}
static void EnterSandbox() {
- // Move the real root to old_root, then detach it.
- char old_root[] = "tmp/old-root-XXXXXX";
- if (mkdtemp(old_root) == NULL) {
- DIE("mkdtemp(%s)", old_root);
- }
-
- // pivot_root has no wrapper in libc, so we need syscall()
- if (syscall(SYS_pivot_root, ".", old_root) < 0) {
- DIE("pivot_root(., %s)", old_root);
- }
-
- if (chroot(".") < 0) {
- DIE("chroot(.)");
- }
-
- if (umount2(old_root, MNT_DETACH) < 0) {
- DIE("umount2(%s, MNT_DETACH)", old_root);
- }
-
- if (rmdir(old_root) < 0) {
- DIE("rmdir(%s)", old_root);
- }
-
if (chdir(opt.working_dir) < 0) {
DIE("chdir(%s)", opt.working_dir);
}
diff --git a/src/main/tools/linux-sandbox.cc b/src/main/tools/linux-sandbox.cc
index 799ece4..0d73174 100644
--- a/src/main/tools/linux-sandbox.cc
+++ b/src/main/tools/linux-sandbox.cc
@@ -71,7 +71,6 @@
int global_outer_uid;
int global_outer_gid;
-static char global_sandbox_root[] = "/tmp/sandbox.XXXXXX";
static int global_child_pid;
// The signal that will be sent to the child when a timeout occurs.
@@ -117,22 +116,6 @@
}
}
-static void RemoveSandboxRoot() {
- if (rmdir(global_sandbox_root) < 0) {
- DIE("rmdir(%s)", global_sandbox_root);
- }
-}
-
-static void SetupSandboxRoot() {
- if (opt.sandbox_root_dir == NULL) {
- if (mkdtemp(global_sandbox_root) == NULL) {
- DIE("mkdtemp(%s)", global_sandbox_root);
- }
- atexit(RemoveSandboxRoot);
- opt.sandbox_root_dir = global_sandbox_root;
- }
-}
-
static void HandleSignal(int signum, void (*handler)(int)) {
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
@@ -283,8 +266,6 @@
// file handles from our parent.
CloseFds();
- SetupSandboxRoot();
-
HandleSignal(SIGALRM, OnTimeout);
if (opt.timeout_secs > 0) {
alarm(opt.timeout_secs);
diff --git a/src/test/shell/bazel/linux-sandbox_test.sh b/src/test/shell/bazel/linux-sandbox_test.sh
index c97f190..87310c3 100755
--- a/src/test/shell/bazel/linux-sandbox_test.sh
+++ b/src/test/shell/bazel/linux-sandbox_test.sh
@@ -28,7 +28,6 @@
readonly OUT="${OUT_DIR}/outfile"
readonly ERR="${OUT_DIR}/errfile"
readonly SANDBOX_DIR="${OUT_DIR}/sandbox"
-readonly SANDBOX_ROOT="${TEST_TMPDIR}/sandbox.root"
readonly MOUNT_TARGET_ROOT="${TEST_SRCDIR}/targets"
SANDBOX_DEFAULT_OPTS="-W $SANDBOX_DIR"
@@ -36,7 +35,6 @@
function set_up {
rm -rf $OUT_DIR
mkdir -p $SANDBOX_DIR
- mkdir -p $SANDBOX_ROOT
}
function test_basic_functionality() {
@@ -122,7 +120,7 @@
touch ${MOUNT_TARGET_ROOT}/sandboxed_testfile
touch /tmp/sandboxed_testfile
- $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D \
-M ${TEST_TMPDIR}/foo -m ${MOUNT_TARGET_ROOT}/foo \
-M ${TEST_TMPDIR}/bar \
-M ${TEST_TMPDIR}/testfile -m ${MOUNT_TARGET_ROOT}/sandboxed_testfile \
@@ -140,7 +138,7 @@
function test_mount_additional_paths_relative_path() {
touch ${TEST_TMPDIR}/testfile
- $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D \
-M ${TEST_TMPDIR}/testfile -m tmp/sandboxed_testfile \
-- /bin/true &> $TEST_log || code=$?
# mount a directory to a customized path inside the sandbox
@@ -150,7 +148,7 @@
function test_mount_additional_paths_leading_m() {
mkdir -p ${TEST_TMPDIR}/foo
touch ${TEST_TMPDIR}/testfile
- $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D \
-m /tmp/foo \
-M ${TEST_TMPDIR}/testfile -m /tmp/sandboxed_testfile \
-- /bin/true &> $TEST_log || code=$?
@@ -162,7 +160,7 @@
mkdir -p ${TEST_TMPDIR}/foo
mkdir -p ${TEST_TMPDIR}/bar
touch ${TEST_TMPDIR}/testfile
- $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D \
-M ${TEST_TMPDIR}/testfile -m /tmp/sandboxed_testfile \
-m /tmp/foo \
-M ${TEST_TMPDIR}/bar \
@@ -174,7 +172,7 @@
function test_mount_additional_paths_other_flag_between_M_m_pair() {
mkdir -p ${TEST_TMPDIR}/bar
touch ${TEST_TMPDIR}/testfile
- $linux_sandbox $SANDBOX_DEFAULT_OPTS -S ${SANDBOX_ROOT} \
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS \
-M ${TEST_TMPDIR}/testfile -D -m /tmp/sandboxed_testfile \
-M ${TEST_TMPDIR}/bar \
-- /bin/true &> $TEST_log || code=$?
@@ -186,7 +184,7 @@
mkdir -p ${TEST_TMPDIR}/foo
mkdir -p ${TEST_TMPDIR}/bar
mkdir -p ${MOUNT_TARGET_ROOT}/foo
- $linux_sandbox $SANDBOX_DEFAULT_OPTS -D -S ${SANDBOX_ROOT} \
+ $linux_sandbox $SANDBOX_DEFAULT_OPTS -D \
-M ${TEST_TMPDIR}/foo -m ${MOUNT_TARGET_ROOT}/foo \
-M ${TEST_TMPDIR}/bar -m ${MOUNT_TARGET_ROOT}/foo \
-- /bin/true &> $TEST_log || code=$?