diff --git a/sandboxed_api/sandbox2/BUILD.bazel b/sandboxed_api/sandbox2/BUILD.bazel index 142489b..40e196d 100644 --- a/sandboxed_api/sandbox2/BUILD.bazel +++ b/sandboxed_api/sandbox2/BUILD.bazel @@ -247,7 +247,6 @@ cc_binary( ":util", "//sandboxed_api/util:raw_logging", "//sandboxed_api/util:strerror", - "@com_google_absl//absl/flags:parse", "@com_google_absl//absl/log:globals", ], ) @@ -607,7 +606,6 @@ 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", @@ -850,12 +848,12 @@ cc_test( data = ["//sandboxed_api/sandbox2/testcases:minimal"], tags = ["no_qemu_user_mode"], deps = [ + ":forkserver", ":forkserver_cc_proto", ":global_forkserver", ":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 0cbfb22..de47282 100644 --- a/sandboxed_api/sandbox2/CMakeLists.txt +++ b/sandboxed_api/sandbox2/CMakeLists.txt @@ -206,8 +206,6 @@ 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 @@ -916,7 +914,6 @@ 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/forkserver.cc b/sandboxed_api/sandbox2/forkserver.cc index 551c973..78b94cf 100644 --- a/sandboxed_api/sandbox2/forkserver.cc +++ b/sandboxed_api/sandbox2/forkserver.cc @@ -39,7 +39,6 @@ #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" -#include "absl/flags/flag.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/match.h" @@ -62,13 +61,6 @@ #include "sandboxed_api/util/raw_logging.h" #include "sandboxed_api/util/strerror.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 { namespace file_util = ::sapi::file_util; @@ -77,18 +69,6 @@ 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. @@ -567,20 +547,11 @@ 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; - 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; - } + sa.sa_handler = SIG_DFL; + sa.sa_flags = SA_NOCLDWAIT; sigemptyset(&sa.sa_mask); if (sigaction(SIGCHLD, &sa, nullptr) == -1) { - 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)"); - } + 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 65f20de..f2208e4 100644 --- a/sandboxed_api/sandbox2/forkserver_bin.cc +++ b/sandboxed_api/sandbox2/forkserver_bin.cc @@ -19,20 +19,16 @@ #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 argc, char* argv[]) { +int main() { // 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/forkserver_test.cc b/sandboxed_api/sandbox2/forkserver_test.cc index a0c0f91..9598c3f 100644 --- a/sandboxed_api/sandbox2/forkserver_test.cc +++ b/sandboxed_api/sandbox2/forkserver_test.cc @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include "sandboxed_api/sandbox2/forkserver.h" + #include #include #include @@ -21,7 +23,6 @@ #include "gtest/gtest.h" #include "absl/log/check.h" -#include "absl/log/log.h" #include "absl/strings/str_cat.h" #include "sandboxed_api/sandbox2/forkserver.pb.h" #include "sandboxed_api/sandbox2/global_forkclient.h" diff --git a/sandboxed_api/sandbox2/global_forkclient.cc b/sandboxed_api/sandbox2/global_forkclient.cc index f0892d4..25d5405 100644 --- a/sandboxed_api/sandbox2/global_forkclient.cc +++ b/sandboxed_api/sandbox2/global_forkclient.cc @@ -53,10 +53,6 @@ #include "sandboxed_api/util/fileops.h" #include "sandboxed_api/util/raw_logging.h" -ABSL_FLAG( - bool, sandbox2_forkserver_use_waitpid, false, - "Use waitpid to reap child processes instead of relying on SA_NOCLDWAIT"); - namespace sandbox2 { namespace file_util = ::sapi::file_util; @@ -128,7 +124,6 @@ GlobalForkserverStartModeSet GetForkserverStartMode() { struct ForkserverArgs { int exec_fd; int comms_fd; - bool use_waitpid; }; int LaunchForkserver(void* vargs) { @@ -147,11 +142,7 @@ int LaunchForkserver(void* vargs) { "duping comms fd failed"); char proc_name[] = "S2-FORK-SERV"; - char use_waitpid[] = "--sandbox2_forkserver_use_waitpid"; - char* argv[] = {proc_name, nullptr, nullptr}; - if (args->use_waitpid) { - argv[1] = use_waitpid; - } + char* const argv[] = {proc_name, nullptr}; util::Execveat(args->exec_fd, "", argv, environ, AT_EMPTY_PATH); SAPI_RAW_PLOG(FATAL, "Could not launch forkserver binary"); } @@ -205,7 +196,6 @@ 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);