Some fixes to process-wrapper / linux-sandbox.
- Refactoring to share more code between the two programs.
- Remove setuid() call in linux-sandbox. It was added due to a wrong understanding of what process-wrapper did in the beginning and unless someone installed linux-sandbox as a setuid binary, it was a no-op.
- Switch to a new process group in linux-sandbox to avoid accidentally killing our parent.
RELNOTES: None.
PiperOrigin-RevId: 156332503
diff --git a/src/main/tools/BUILD b/src/main/tools/BUILD
index a29b71f..72aff63 100644
--- a/src/main/tools/BUILD
+++ b/src/main/tools/BUILD
@@ -1,11 +1,16 @@
package(default_visibility = ["//src:__subpackages__"])
cc_library(
+ name = "logging",
+ srcs = ["logging.cc"],
+ hdrs = ["logging.h"],
+)
+
+cc_library(
name = "process-tools",
- srcs = [
- "process-tools.cc",
- "process-tools.h",
- ],
+ srcs = ["process-tools.cc"],
+ hdrs = ["process-tools.h"],
+ deps = [":logging"],
)
cc_binary(
@@ -21,6 +26,7 @@
"//src:windows_msvc": [],
"//conditions:default": [
":process-tools",
+ ":logging",
],
}),
)
@@ -49,10 +55,21 @@
"linux-sandbox-options.h",
"linux-sandbox-pid1.cc",
"linux-sandbox-pid1.h",
- "linux-sandbox-utils.h",
],
}),
linkopts = ["-lm"],
+ deps = select({
+ "//src:darwin": [],
+ "//src:darwin_x86_64": [],
+ "//src:freebsd": [],
+ "//src:windows": [],
+ "//src:windows_msys": [],
+ "//src:windows_msvc": [],
+ "//conditions:default": [
+ ":logging",
+ ":process-tools",
+ ],
+ }),
)
filegroup(
diff --git a/src/main/tools/linux-sandbox-options.cc b/src/main/tools/linux-sandbox-options.cc
index c646cec..ce4a624 100644
--- a/src/main/tools/linux-sandbox-options.cc
+++ b/src/main/tools/linux-sandbox-options.cc
@@ -12,14 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#define DIE(args...) \
- { \
- fprintf(stderr, __FILE__ ":" S__LINE__ ": \"" args); \
- fprintf(stderr, "\": "); \
- perror(nullptr); \
- exit(EXIT_FAILURE); \
- }
-
#include "src/main/tools/linux-sandbox-options.h"
#include <errno.h>
@@ -37,7 +29,8 @@
#include <string>
#include <vector>
-#include "src/main/tools/linux-sandbox-utils.h"
+#include "src/main/tools/logging.h"
+#include "src/main/tools/process-tools.h"
using std::ifstream;
using std::unique_ptr;
@@ -207,8 +200,8 @@
// Expands a single argument, expanding options @filename to read in the content
// of the file and add it to the list of processed arguments.
-unique_ptr<vector<char *>> ExpandArgument(unique_ptr<vector<char *>> expanded,
- char *arg) {
+static unique_ptr<vector<char *>> ExpandArgument(
+ unique_ptr<vector<char *>> expanded, char *arg) {
if (arg[0] == '@') {
const char *filename = arg + 1; // strip off the '@'.
ifstream f(filename);
@@ -236,7 +229,7 @@
// Pre-processes an argument list, expanding options @filename to read in the
// content of the file and add it to the list of arguments. Stops expanding
// arguments once it encounters "--".
-unique_ptr<vector<char *>> ExpandArguments(const vector<char *> &args) {
+static unique_ptr<vector<char *>> ExpandArguments(const vector<char *> &args) {
unique_ptr<vector<char *>> expanded(new vector<char *>());
expanded->reserve(args.size());
for (auto arg = args.begin(); arg != args.end(); ++arg) {
@@ -250,7 +243,6 @@
return expanded;
}
-// Handles parsing all command line flags and populates the global opt struct.
void ParseOptions(int argc, char *argv[]) {
vector<char *> args(argv, argv + argc);
ParseCommandLine(ExpandArguments(args));
diff --git a/src/main/tools/linux-sandbox-options.h b/src/main/tools/linux-sandbox-options.h
index d67a14b..c190372 100644
--- a/src/main/tools/linux-sandbox-options.h
+++ b/src/main/tools/linux-sandbox-options.h
@@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#ifndef LINUX_SANDBOX_OPTIONS_H__
-#define LINUX_SANDBOX_OPTIONS_H__
+#ifndef SRC_MAIN_TOOLS_LINUX_SANDBOX_OPTIONS_H_
+#define SRC_MAIN_TOOLS_LINUX_SANDBOX_OPTIONS_H_
#include <stdbool.h>
#include <stddef.h>
@@ -56,6 +56,7 @@
extern struct Options opt;
+// Handles parsing all command line flags and populates the global opt struct.
void ParseOptions(int argc, char *argv[]);
#endif
diff --git a/src/main/tools/linux-sandbox-pid1.cc b/src/main/tools/linux-sandbox-pid1.cc
index 740c8de..5c923e9 100644
--- a/src/main/tools/linux-sandbox-pid1.cc
+++ b/src/main/tools/linux-sandbox-pid1.cc
@@ -17,20 +17,7 @@
* mount, UTS, IPC and PID namespace.
*/
-#include "src/main/tools/linux-sandbox-options.h"
-#include "src/main/tools/linux-sandbox-utils.h"
-#include "src/main/tools/linux-sandbox.h"
-
-// Note that we define DIE() here and not in a shared header, because we want to
-// use _exit() in the
-// pid1 child, but exit() in the parent.
-#define DIE(args...) \
- { \
- fprintf(stderr, __FILE__ ":" S__LINE__ ": \"" args); \
- fprintf(stderr, "\": "); \
- perror(nullptr); \
- _exit(EXIT_FAILURE); \
- }
+#include "src/main/tools/linux-sandbox-pid1.h"
#include <errno.h>
#include <fcntl.h>
@@ -53,9 +40,13 @@
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
-
#include <string>
+#include "src/main/tools/linux-sandbox-options.h"
+#include "src/main/tools/linux-sandbox.h"
+#include "src/main/tools/logging.h"
+#include "src/main/tools/process-tools.h"
+
static int global_child_pid;
static void SetupSelfDestruction(int *sync_pipe) {
@@ -68,6 +59,13 @@
DIE("prctl");
}
+ // Switch to a new process group, otherwise our process group will still refer
+ // to the outer PID namespace. We might then accidentally kill our parent by a
+ // call to e.g. `kill(0, sig)`.
+ if (setpgid(0, 0) < 0) {
+ DIE("setpgid");
+ }
+
// Verify that the parent still lives.
char buf = 0;
if (close(sync_pipe[0]) < 0) {
@@ -301,8 +299,7 @@
DIE("socket");
}
- struct ifreq ifr;
- memset(&ifr, 0, sizeof(ifr));
+ struct ifreq ifr = {};
strncpy(ifr.ifr_name, "lo", IF_NAMESIZE);
// Verify that name is valid.
@@ -329,8 +326,7 @@
}
static void InstallSignalHandler(int signum, void (*handler)(int)) {
- struct sigaction sa;
- memset(&sa, 0, sizeof(sa));
+ struct sigaction sa = {};
sa.sa_handler = handler;
if (handler == SIG_IGN || handler == SIG_DFL) {
// No point in blocking signals when using the default handler or ignoring
@@ -366,8 +362,7 @@
}
// Set the default signal handler for all signals.
- struct sigaction sa;
- memset(&sa, 0, sizeof(sa));
+ struct sigaction sa = {};
if (sigemptyset(&sa.sa_mask) < 0) {
DIE("sigemptyset");
}
diff --git a/src/main/tools/linux-sandbox-pid1.h b/src/main/tools/linux-sandbox-pid1.h
index 507739a..a8260db 100644
--- a/src/main/tools/linux-sandbox-pid1.h
+++ b/src/main/tools/linux-sandbox-pid1.h
@@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#ifndef LINUX_SANDBOX_PID1_H__
-#define LINUX_SANDBOX_PID1_H__
+#ifndef SRC_MAIN_TOOLS_LINUX_SANDBOX_PID1_H_
+#define SRC_MAIN_TOOLS_LINUX_SANDBOX_PID1_H_
int Pid1Main(void *sync_pipe_param);
diff --git a/src/main/tools/linux-sandbox-utils.h b/src/main/tools/linux-sandbox-utils.h
deleted file mode 100644
index 3a4a1bf..0000000
--- a/src/main/tools/linux-sandbox-utils.h
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright 2016 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#ifndef LINUX_SANDBOX_UTILS_H__
-#define LINUX_SANDBOX_UTILS_H__
-
-#define S(x) #x
-#define S_(x) S(x)
-#define S__LINE__ S_(__LINE__)
-
-#define PRINT_DEBUG(...) \
- do { \
- if (opt.debug) { \
- fprintf(stderr, __FILE__ ":" S__LINE__ ": " __VA_ARGS__); \
- fprintf(stderr, "\n"); \
- } \
- } while (0)
-
-#endif
diff --git a/src/main/tools/linux-sandbox.cc b/src/main/tools/linux-sandbox.cc
index b58905e..0326d24 100644
--- a/src/main/tools/linux-sandbox.cc
+++ b/src/main/tools/linux-sandbox.cc
@@ -37,13 +37,7 @@
* system are invisible.
*/
-#define DIE(args...) \
- { \
- fprintf(stderr, __FILE__ ":" S__LINE__ ": \"" args); \
- fprintf(stderr, "\": "); \
- perror(nullptr); \
- exit(EXIT_FAILURE); \
- }
+#include "src/main/tools/linux-sandbox.h"
#include <ctype.h>
#include <dirent.h>
@@ -67,7 +61,8 @@
#include "src/main/tools/linux-sandbox-options.h"
#include "src/main/tools/linux-sandbox-pid1.h"
-#include "src/main/tools/linux-sandbox-utils.h"
+#include "src/main/tools/logging.h"
+#include "src/main/tools/process-tools.h"
int global_outer_uid;
int global_outer_gid;
@@ -80,6 +75,8 @@
// The signal that caused us to kill the child (e.g. on timeout).
static volatile sig_atomic_t global_signal;
+// Make sure the child process does not inherit any accidentally left open file
+// handles from our parent.
static void CloseFds() {
DIR *fds = opendir("/proc/self/fd");
if (fds == nullptr) {
@@ -117,18 +114,6 @@
}
}
-static void HandleSignal(int signum, void (*handler)(int)) {
- struct sigaction sa;
- memset(&sa, 0, sizeof(sa));
- sa.sa_handler = handler;
- if (sigemptyset(&sa.sa_mask) < 0) {
- DIE("sigemptyset");
- }
- if (sigaction(signum, &sa, nullptr) < 0) {
- DIE("sigaction");
- }
-}
-
static void OnTimeout(int sig) {
global_signal = sig;
kill(global_child_pid, global_next_timeout_signal);
@@ -210,31 +195,6 @@
}
}
-static void Redirect(const std::string &target_path, int fd) {
- if (!target_path.empty() && target_path != "-") {
- const int flags = O_WRONLY | O_CREAT | O_TRUNC | O_APPEND;
- int fd_out = open(target_path.c_str(), flags, 0666);
- if (fd_out < 0) {
- DIE("open(%s)", target_path.c_str());
- }
- // If we were launched with less than 3 fds (stdin, stdout, stderr) open,
- // but redirection is still requested via a command-line flag, something is
- // wacky and the following code would not do what we intend to do, so let's
- // bail.
- if (fd_out < 3) {
- DIE("open(%s) returned a handle that is reserved for stdin / stdout / "
- "stderr",
- target_path.c_str());
- }
- if (dup2(fd_out, fd) < 0) {
- DIE("dup2()");
- }
- if (close(fd_out) < 0) {
- DIE("close()");
- }
- }
-}
-
int main(int argc, char *argv[]) {
// Ask the kernel to kill us with SIGKILL if our parent dies.
if (prctl(PR_SET_PDEATHSIG, SIGKILL) < 0) {
@@ -242,26 +202,19 @@
}
ParseOptions(argc, argv);
+ global_debug = opt.debug;
Redirect(opt.stdout_path, STDOUT_FILENO);
Redirect(opt.stderr_path, STDERR_FILENO);
- // This should never be called as a setuid binary, drop privileges just in
- // case. We don't need to be root, because we use user namespaces anyway.
- if (setuid(getuid()) < 0) {
- DIE("setuid");
- }
-
global_outer_uid = getuid();
global_outer_gid = getgid();
- // Make sure the sandboxed process does not inherit any accidentally left open
- // file handles from our parent.
CloseFds();
- HandleSignal(SIGALRM, OnTimeout);
if (opt.timeout_secs > 0) {
- alarm(opt.timeout_secs);
+ HandleSignal(SIGALRM, OnTimeout);
+ SetTimeout(opt.timeout_secs);
}
SpawnPid1();
diff --git a/src/main/tools/linux-sandbox.h b/src/main/tools/linux-sandbox.h
index df07dee..18a744c 100644
--- a/src/main/tools/linux-sandbox.h
+++ b/src/main/tools/linux-sandbox.h
@@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#ifndef LINUX_SANDBOX_H__
-#define LINUX_SANDBOX_H__
+#ifndef SRC_MAIN_TOOLS_LINUX_SANDBOX_H_
+#define SRC_MAIN_TOOLS_LINUX_SANDBOX_H_
extern int global_outer_uid;
extern int global_outer_gid;
diff --git a/src/main/tools/logging.cc b/src/main/tools/logging.cc
new file mode 100644
index 0000000..d2a1ca8
--- /dev/null
+++ b/src/main/tools/logging.cc
@@ -0,0 +1,17 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/main/tools/logging.h"
+
+bool global_debug = false;
diff --git a/src/main/tools/logging.h b/src/main/tools/logging.h
new file mode 100644
index 0000000..be646a7
--- /dev/null
+++ b/src/main/tools/logging.h
@@ -0,0 +1,43 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef SRC_MAIN_TOOLS_LOGGING_H_
+#define SRC_MAIN_TOOLS_LOGGING_H_
+
+// see
+// http://stackoverflow.com/questions/5641427/how-to-make-preprocessor-generate-a-string-for-line-keyword
+#define S(x) #x
+#define S_(x) S(x)
+#define S__LINE__ S_(__LINE__)
+
+#define DIE(...) \
+ { \
+ fprintf(stderr, __FILE__ ":" S__LINE__ ": \"" __VA_ARGS__); \
+ fprintf(stderr, "\": "); \
+ perror(nullptr); \
+ exit(EXIT_FAILURE); \
+ }
+
+#define PRINT_DEBUG(...) \
+ do { \
+ if (global_debug) { \
+ fprintf(stderr, __FILE__ ":" S__LINE__ ": " __VA_ARGS__); \
+ fprintf(stderr, "\n"); \
+ } \
+ } while (0)
+
+// Set to `true` to let PRINT_DEBUG() print messages.
+extern bool global_debug;
+
+#endif // SRC_MAIN_TOOLS_LOGGING_H_
diff --git a/src/main/tools/process-tools.cc b/src/main/tools/process-tools.cc
index a25dbee..0c77611 100644
--- a/src/main/tools/process-tools.cc
+++ b/src/main/tools/process-tools.cc
@@ -27,7 +27,7 @@
#include <sys/wait.h>
#include <unistd.h>
-extern bool global_debug;
+#include "src/main/tools/logging.h"
int SwitchToEuid() {
int uid = getuid();
@@ -143,10 +143,6 @@
}
void SetTimeout(double timeout_secs) {
- if (timeout_secs <= 0) {
- return;
- }
-
double int_val, fraction_val;
fraction_val = modf(timeout_secs, &int_val);
diff --git a/src/main/tools/process-tools.h b/src/main/tools/process-tools.h
index 7a8554d..ef8f3fb 100644
--- a/src/main/tools/process-tools.h
+++ b/src/main/tools/process-tools.h
@@ -12,35 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#ifndef PROCESS_TOOLS_H__
-#define PROCESS_TOOLS_H__
+#ifndef SRC_MAIN_TOOLS_PROCESS_TOOLS_H_
+#define SRC_MAIN_TOOLS_PROCESS_TOOLS_H_
#include <stdbool.h>
#include <sys/types.h>
#include <string>
-// see
-// http://stackoverflow.com/questions/5641427/how-to-make-preprocessor-generate-a-string-for-line-keyword
-#define S(x) #x
-#define S_(x) S(x)
-#define S__LINE__ S_(__LINE__)
-
-#define DIE(...) \
- { \
- fprintf(stderr, __FILE__ ":" S__LINE__ ": \"" __VA_ARGS__); \
- fprintf(stderr, "\": "); \
- perror(nullptr); \
- exit(EXIT_FAILURE); \
- }
-
-#define PRINT_DEBUG(...) \
- do { \
- if (global_debug) { \
- fprintf(stderr, __FILE__ ":" S__LINE__ ": " __VA_ARGS__); \
- fprintf(stderr, "\n"); \
- } \
- } while (0)
-
// Switch completely to the effective uid.
// Some programs (notably, bash) ignore the euid and just use the uid. This
// limits the ability for us to use process-wrapper as a setuid binary for
diff --git a/src/main/tools/process-wrapper.cc b/src/main/tools/process-wrapper.cc
index 6140eba..c2eeee2 100644
--- a/src/main/tools/process-wrapper.cc
+++ b/src/main/tools/process-wrapper.cc
@@ -36,10 +36,9 @@
#include <string>
#include <vector>
+#include "src/main/tools/logging.h"
#include "src/main/tools/process-tools.h"
-bool global_debug = false;
-
static double global_kill_delay;
static pid_t global_child_pid;
static volatile sig_atomic_t global_signal;
@@ -136,7 +135,9 @@
HandleSignal(SIGALRM, OnSignal);
HandleSignal(SIGTERM, OnSignal);
HandleSignal(SIGINT, OnSignal);
- SetTimeout(timeout_secs);
+ if (timeout_secs > 0) {
+ SetTimeout(timeout_secs);
+ }
int status = WaitChild(global_child_pid);
