Fix rare failure while starting the global forkserver

This bug only manifests if a lot of fds are open when global forkserver is started.
If the allocated exec_fd number was equal Comms::kSandbox2ClientCommsFD then it would be replaced by the comms fd and result in EACCESS at execveat.

PiperOrigin-RevId: 380805414
Change-Id: I31427fa929abfc60890477b55790cc14c749f7f5
This commit is contained in:
Wiktor Garbacz 2021-06-22 07:48:25 -07:00 committed by Copybara-Service
parent a850aa44d2
commit 0ec4f07f96
3 changed files with 14 additions and 10 deletions

View File

@ -225,7 +225,6 @@ cc_library(
":forkserver_cc_proto",
":util",
"//sandboxed_api:embed_file",
"//sandboxed_api/util:fileops",
"//sandboxed_api/util:flags",
"//sandboxed_api/util:raw_logging",
"//sandboxed_api/util:strerror",

View File

@ -226,7 +226,6 @@ target_link_libraries(sandbox2_global_forkserver
absl::strings
glog::glog
sandbox2::client
sapi::fileops
sandbox2::forkserver_bin_embed
sapi::strerror
sandbox2::util

View File

@ -41,14 +41,11 @@
#include "sandboxed_api/sandbox2/fork_client.h"
#include "sandboxed_api/sandbox2/forkserver_bin_embed.h"
#include "sandboxed_api/sandbox2/util.h"
#include "sandboxed_api/util/fileops.h"
#include "sandboxed_api/util/raw_logging.h"
#include "sandboxed_api/util/strerror.h"
namespace sandbox2 {
namespace file_util = ::sapi::file_util;
bool AbslParseFlag(absl::string_view text, GlobalForkserverStartModeSet* out,
std::string* error) {
*out = {};
@ -120,10 +117,10 @@ GlobalForkserverStartModeSet GetForkserverStartMode() {
}
std::unique_ptr<GlobalForkClient> StartGlobalForkServer() {
file_util::fileops::FDCloser exec_fd(
sapi::EmbedFile::GetEmbedFileSingleton()->GetDupFdForFileToc(
forkserver_bin_embed_create()));
SAPI_RAW_CHECK(exec_fd.get() >= 0, "Getting FD for init binary failed");
// The fd is owned by EmbedFile
int exec_fd = sapi::EmbedFile::GetEmbedFileSingleton()->GetFdForFileToc(
forkserver_bin_embed_create());
SAPI_RAW_CHECK(exec_fd >= 0, "Getting FD for init binary failed");
std::string proc_name = "S2-FORK-SERV";
@ -139,11 +136,20 @@ std::unique_ptr<GlobalForkClient> StartGlobalForkServer() {
if (pid == 0) {
// Move the comms FD to the proper, expected FD number.
// The new FD will not be CLOEXEC, which is what we want.
// If exec_fd == Comms::kSandbox2ClientCommsFD then it would be replaced by
// the comms fd and result in EACCESS at execveat.
// So first move exec_fd also making sure it will not clash with sv[0]...
int new_exec_fd = Comms::kSandbox2ClientCommsFD + 1;
if (sv[0] == new_exec_fd) {
++new_exec_fd;
}
dup2(exec_fd, new_exec_fd);
fcntl(new_exec_fd, F_SETFD, FD_CLOEXEC);
dup2(sv[0], Comms::kSandbox2ClientCommsFD);
char* const args[] = {proc_name.data(), nullptr};
char* const envp[] = {nullptr};
syscall(__NR_execveat, exec_fd.get(), "", args, envp, AT_EMPTY_PATH);
syscall(__NR_execveat, new_exec_fd, "", args, envp, AT_EMPTY_PATH);
SAPI_RAW_PLOG(FATAL, "Could not launch forkserver binary");
abort();
}