Test for races in IptablesRestoreController::Init.
Bug: 28362720
Test: angler builds, boots
Test: netd_{unit,integration}_test pass
Change-Id: I73ed28c7e7edaeb65a3b346b89ec69f472fd5974
diff --git a/server/IptablesRestoreController.cpp b/server/IptablesRestoreController.cpp
index e346b82..a90224a 100644
--- a/server/IptablesRestoreController.cpp
+++ b/server/IptablesRestoreController.cpp
@@ -24,9 +24,13 @@
#define LOG_TAG "IptablesRestoreController"
#include <android-base/logging.h>
#include <android-base/file.h>
+#include <netdutils/Syscalls.h>
#include "Controllers.h"
+using android::netdutils::StatusOr;
+using android::netdutils::sSyscalls;
+
constexpr char IPTABLES_RESTORE_PATH[] = "/system/bin/iptables-restore";
constexpr char IP6TABLES_RESTORE_PATH[] = "/system/bin/ip6tables-restore";
@@ -113,6 +117,13 @@
};
IptablesRestoreController::IptablesRestoreController() {
+ Init();
+}
+
+IptablesRestoreController::~IptablesRestoreController() {
+}
+
+void IptablesRestoreController::Init() {
// Start the IPv4 and IPv6 processes in parallel, since each one takes 20-30ms.
std::thread v4([this] () { mIpRestore.reset(forkAndExec(IPTABLES_PROCESS)); });
std::thread v6([this] () { mIp6Restore.reset(forkAndExec(IP6TABLES_PROCESS)); });
@@ -120,9 +131,6 @@
v6.join();
}
-IptablesRestoreController::~IptablesRestoreController() {
-}
-
/* static */
IptablesProcess* IptablesRestoreController::forkAndExec(const IptablesProcessType type) {
const char* const cmd = (type == IPTABLES_PROCESS) ?
@@ -142,8 +150,14 @@
return nullptr;
}
- pid_t child_pid = fork();
- if (child_pid == 0) {
+ const auto& sys = sSyscalls.get();
+ StatusOr<pid_t> child_pid = sys.fork();
+ if (!isOk(child_pid)) {
+ ALOGE("fork() failed: %s", strerror(child_pid.status().code()));
+ return nullptr;
+ }
+
+ if (child_pid.value() == 0) {
// The child process. Reads from stdin, writes to stderr and stdout.
// stdin_pipe[1] : The write end of the stdin pipe.
@@ -183,11 +197,6 @@
}
// The parent process. Writes to stdout and stderr and reads from stdin.
- if (child_pid == -1) {
- ALOGE("fork() failed: %s", strerror(errno));
- return nullptr;
- }
-
// stdin_pipe[0] : The read end of the stdin pipe.
// stdout_pipe[1] : The write end of the stdout pipe.
// stderr_pipe[1] : The write end of the stderr pipe.
@@ -197,7 +206,7 @@
ALOGW("close() failed: %s", strerror(errno));
}
- return new IptablesProcess(child_pid, stdin_pipe[1], stdout_pipe[0], stderr_pipe[0]);
+ return new IptablesProcess(child_pid.value(), stdin_pipe[1], stdout_pipe[0], stderr_pipe[0]);
}
// TODO: Return -errno on failure instead of -1.