diff --git a/sandboxed_api/sandbox2/BUILD.bazel b/sandboxed_api/sandbox2/BUILD.bazel index 2adde00..0cb15ef 100644 --- a/sandboxed_api/sandbox2/BUILD.bazel +++ b/sandboxed_api/sandbox2/BUILD.bazel @@ -268,6 +268,7 @@ cc_binary( ":util", "//sandboxed_api/util:raw_logging", "//sandboxed_api/util:strerror", + "@com_google_absl//absl/flags:parse", "@com_google_absl//absl/log:globals", ], ) @@ -628,6 +629,7 @@ cc_library( "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/flags:flag", "@com_google_absl//absl/log", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", @@ -647,6 +649,7 @@ cc_library( ":forkserver_cc_proto", "//sandboxed_api/util:fileops", "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/flags:flag", "@com_google_absl//absl/log", "@com_google_absl//absl/log:check", "@com_google_absl//absl/synchronization", @@ -876,6 +879,7 @@ cc_test( ":sandbox2", "//sandboxed_api:testing", "//sandboxed_api/util:raw_logging", + "@com_google_absl//absl/log", "@com_google_absl//absl/log:check", "@com_google_absl//absl/strings", "@com_google_googletest//:gtest_main", diff --git a/sandboxed_api/sandbox2/CMakeLists.txt b/sandboxed_api/sandbox2/CMakeLists.txt index acd8748..a1d2289 100644 --- a/sandboxed_api/sandbox2/CMakeLists.txt +++ b/sandboxed_api/sandbox2/CMakeLists.txt @@ -215,6 +215,8 @@ set_target_properties(sandbox2_forkserver_bin PROPERTIES add_executable(sandbox2::forkserver_bin ALIAS sandbox2_forkserver_bin) target_link_libraries(sandbox2_forkserver_bin PRIVATE absl::log_globals + absl::flags + absl::flags_parse sandbox2::client sandbox2::comms sandbox2::forkserver @@ -590,6 +592,7 @@ target_link_libraries(sandbox2_fork_client PRIVATE sandbox2::comms sandbox2::forkserver_proto PUBLIC absl::core_headers + absl::flags absl::synchronization sapi::base sapi::fileops @@ -924,6 +927,7 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING) ) target_link_libraries(sandbox2_forkserver_test PRIVATE absl::check + absl::log absl::strings sandbox2::forkserver sandbox2::forkserver_proto diff --git a/sandboxed_api/sandbox2/fork_client.cc b/sandboxed_api/sandbox2/fork_client.cc index d40c8f8..d25ee69 100644 --- a/sandboxed_api/sandbox2/fork_client.cc +++ b/sandboxed_api/sandbox2/fork_client.cc @@ -14,11 +14,19 @@ #include "sandboxed_api/sandbox2/fork_client.h" +#include "absl/flags/flag.h" #include "absl/log/check.h" #include "absl/log/log.h" #include "sandboxed_api/sandbox2/comms.h" #include "sandboxed_api/sandbox2/forkserver.pb.h" +// Make the forkserver use a signal handler for SIGCHLD instead of setting +// `SA_NOCLDWAIT`. This is needed in certain cases where the process need +// to be explicitly reaped by waitpid (see getrusage). +ABSL_FLAG( + bool, sandbox2_forkserver_use_waitpid, false, + "Use waitpid to reap child processes instead of relying on SA_NOCLDWAIT"); + namespace sandbox2 { using ::sapi::file_util::fileops::FDCloser; diff --git a/sandboxed_api/sandbox2/forkserver.cc b/sandboxed_api/sandbox2/forkserver.cc index 78b94cf..d92418e 100644 --- a/sandboxed_api/sandbox2/forkserver.cc +++ b/sandboxed_api/sandbox2/forkserver.cc @@ -39,6 +39,8 @@ #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" +#include "absl/flags/declare.h" +#include "absl/flags/flag.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/match.h" @@ -61,6 +63,8 @@ #include "sandboxed_api/util/raw_logging.h" #include "sandboxed_api/util/strerror.h" +ABSL_DECLARE_FLAG(bool, sandbox2_forkserver_use_waitpid); + namespace sandbox2 { namespace file_util = ::sapi::file_util; @@ -69,6 +73,18 @@ namespace { using ::sapi::StrError; +void SigChldHandler(int signal) { + if (signal != SIGCHLD) { + return; + } + + pid_t pid; + int wstatus; + do { + pid = waitpid(-1, &wstatus, WNOHANG); + } while (pid > 0); +} + // "Moves" FDs in move_fds from current to target FD number while keeping FDs // in keep_fds open - potentially moving them to another FD number as well in // case of colisions. @@ -547,11 +563,20 @@ bool ForkServer::Initialize() { // Don't convert terminated child processes into zombies. It's up to the // sandbox (Monitor) to track them and receive/report their final status. struct sigaction sa; - sa.sa_handler = SIG_DFL; - sa.sa_flags = SA_NOCLDWAIT; + if (absl::GetFlag(FLAGS_sandbox2_forkserver_use_waitpid)) { + sa.sa_handler = SigChldHandler; + sa.sa_flags = 0; + } else { + sa.sa_handler = SIG_DFL; + sa.sa_flags = SA_NOCLDWAIT; + } sigemptyset(&sa.sa_mask); if (sigaction(SIGCHLD, &sa, nullptr) == -1) { - SAPI_RAW_PLOG(ERROR, "sigaction(SIGCHLD, flags=SA_NOCLDWAIT)"); + if (absl::GetFlag(FLAGS_sandbox2_forkserver_use_waitpid)) { + SAPI_RAW_PLOG(ERROR, "sigaction(SIGCHLD, sa_handler=SigChldHandler)"); + } else { + SAPI_RAW_PLOG(ERROR, "sigaction(SIGCHLD, flags=SA_NOCLDWAIT)"); + } return false; } return true; diff --git a/sandboxed_api/sandbox2/forkserver_bin.cc b/sandboxed_api/sandbox2/forkserver_bin.cc index f2208e4..65f20de 100644 --- a/sandboxed_api/sandbox2/forkserver_bin.cc +++ b/sandboxed_api/sandbox2/forkserver_bin.cc @@ -19,16 +19,20 @@ #include #include +#include "absl/flags/parse.h" #include "absl/log/globals.h" #include "sandboxed_api/sandbox2/comms.h" #include "sandboxed_api/sandbox2/forkserver.h" #include "sandboxed_api/sandbox2/sanitizer.h" #include "sandboxed_api/util/raw_logging.h" -int main() { +int main(int argc, char* argv[]) { // Make sure the logs go stderr. absl::SetStderrThreshold(absl::LogSeverityAtLeast::kInfo); + // Parse command line arguments. + absl::ParseCommandLine(argc, argv); + // Close all non-essential FDs to keep newly opened FD numbers consistent. absl::Status status = sandbox2::sanitizer::CloseAllFDsExcept( {0, 1, 2, sandbox2::Comms::kSandbox2ClientCommsFD}); diff --git a/sandboxed_api/sandbox2/global_forkclient.cc b/sandboxed_api/sandbox2/global_forkclient.cc index 25d5405..fc02e35 100644 --- a/sandboxed_api/sandbox2/global_forkclient.cc +++ b/sandboxed_api/sandbox2/global_forkclient.cc @@ -34,6 +34,7 @@ #include #include "absl/cleanup/cleanup.h" +#include "absl/flags/declare.h" #include "absl/flags/flag.h" #include "absl/log/log.h" #include "absl/status/status.h" @@ -53,6 +54,8 @@ #include "sandboxed_api/util/fileops.h" #include "sandboxed_api/util/raw_logging.h" +ABSL_DECLARE_FLAG(bool, sandbox2_forkserver_use_waitpid); + namespace sandbox2 { namespace file_util = ::sapi::file_util; @@ -124,6 +127,7 @@ GlobalForkserverStartModeSet GetForkserverStartMode() { struct ForkserverArgs { int exec_fd; int comms_fd; + bool use_waitpid; }; int LaunchForkserver(void* vargs) { @@ -142,7 +146,11 @@ int LaunchForkserver(void* vargs) { "duping comms fd failed"); char proc_name[] = "S2-FORK-SERV"; - char* const argv[] = {proc_name, nullptr}; + char use_waitpid[] = "--sandbox2_forkserver_use_waitpid"; + char* argv[] = {proc_name, nullptr, nullptr}; + if (args->use_waitpid) { + argv[1] = use_waitpid; + } util::Execveat(args->exec_fd, "", argv, environ, AT_EMPTY_PATH); SAPI_RAW_PLOG(FATAL, "Could not launch forkserver binary"); } @@ -196,6 +204,7 @@ absl::StatusOr> StartGlobalForkServer() { ForkserverArgs args = { .exec_fd = exec_fd, .comms_fd = sv[0], + .use_waitpid = absl::GetFlag(FLAGS_sandbox2_forkserver_use_waitpid), }; pid_t pid = clone(LaunchForkserver, &stack[stack_size], clone_flags, &args, nullptr, nullptr, nullptr);