Merge "Fix posix_spawn signal defaulting."
diff --git a/libc/bionic/spawn.cpp b/libc/bionic/spawn.cpp
index 061d68c..e5075f5 100644
--- a/libc/bionic/spawn.cpp
+++ b/libc/bionic/spawn.cpp
@@ -89,39 +89,47 @@
int schedpolicy;
sigset_t sigmask;
sigset_t sigdefault;
-
- void Do() {
- if ((flags & POSIX_SPAWN_SETSIGDEF) != 0) {
- // POSIX: "If POSIX_SPAWN_SETSIGDEF is set ... signals in sigdefault ... shall be set to
- // their default actions in the child process."
- struct sigaction sa = {};
- sa.sa_handler = SIG_DFL;
- for (int s = 1; s < _NSIG; ++s) {
- if (sigismember(&sigdefault, s) && sigaction(s, &sa, nullptr) == -1) _exit(127);
- }
- }
-
- if ((flags & POSIX_SPAWN_SETPGROUP) != 0 && setpgid(0, pgroup) == -1) _exit(127);
- if ((flags & POSIX_SPAWN_SETSID) != 0 && setsid() == -1) _exit(127);
-
- // POSIX_SPAWN_SETSCHEDULER overrides POSIX_SPAWN_SETSCHEDPARAM, but it is not an error
- // to set both.
- if ((flags & POSIX_SPAWN_SETSCHEDULER) != 0) {
- if (sched_setscheduler(0, schedpolicy, &schedparam) == -1) _exit(127);
- } else if ((flags & POSIX_SPAWN_SETSCHEDPARAM) != 0) {
- if (sched_setparam(0, &schedparam) == -1) _exit(127);
- }
-
- if ((flags & POSIX_SPAWN_RESETIDS) != 0) {
- if (seteuid(getuid()) == -1 || setegid(getgid()) == -1) _exit(127);
- }
-
- if ((flags & POSIX_SPAWN_SETSIGMASK) != 0) {
- if (sigprocmask(SIG_SETMASK, &sigmask, nullptr)) _exit(127);
- }
- }
};
+static void ApplyAttrs(short flags, const posix_spawnattr_t* attr) {
+ // POSIX: "If POSIX_SPAWN_SETSIGDEF is set ... signals in sigdefault ...
+ // shall be set to their default actions in the child process."
+ // POSIX: "Signals set to be caught by the calling process shall be
+ // set to the default action in the child process."
+ bool use_sigdefault = ((flags & POSIX_SPAWN_SETSIGDEF) != 0);
+ const struct sigaction default_sa = { .sa_handler = SIG_DFL };
+ for (int s = 1; s < _NSIG; ++s) {
+ bool reset = false;
+ if (use_sigdefault && sigismember(&(*attr)->sigdefault, s)) {
+ reset = true;
+ } else {
+ struct sigaction current;
+ if (sigaction(s, nullptr, ¤t) == -1) _exit(127);
+ reset = (current.sa_handler != SIG_IGN && current.sa_handler != SIG_DFL);
+ }
+ if (reset && sigaction(s, &default_sa, nullptr) == -1) _exit(127);
+ }
+
+ if ((flags & POSIX_SPAWN_SETPGROUP) != 0 && setpgid(0, (*attr)->pgroup) == -1) _exit(127);
+ if ((flags & POSIX_SPAWN_SETSID) != 0 && setsid() == -1) _exit(127);
+
+ // POSIX_SPAWN_SETSCHEDULER overrides POSIX_SPAWN_SETSCHEDPARAM, but it is not an error
+ // to set both.
+ if ((flags & POSIX_SPAWN_SETSCHEDULER) != 0) {
+ if (sched_setscheduler(0, (*attr)->schedpolicy, &(*attr)->schedparam) == -1) _exit(127);
+ } else if ((flags & POSIX_SPAWN_SETSCHEDPARAM) != 0) {
+ if (sched_setparam(0, &(*attr)->schedparam) == -1) _exit(127);
+ }
+
+ if ((flags & POSIX_SPAWN_RESETIDS) != 0) {
+ if (seteuid(getuid()) == -1 || setegid(getgid()) == -1) _exit(127);
+ }
+
+ if ((flags & POSIX_SPAWN_SETSIGMASK) != 0) {
+ if (sigprocmask(SIG_SETMASK, &(*attr)->sigmask, nullptr)) _exit(127);
+ }
+}
+
static int posix_spawn(pid_t* pid_ptr,
const char* path,
const posix_spawn_file_actions_t* actions,
@@ -142,7 +150,7 @@
if (pid == 0) {
// Child.
- if (attr) (*attr)->Do();
+ ApplyAttrs(flags, attr);
if (actions) (*actions)->Do();
if ((flags & POSIX_SPAWN_SETSIGMASK) == 0) ssb.reset();
exec_fn(path, argv, env ? env : environ);
diff --git a/tests/spawn_test.cpp b/tests/spawn_test.cpp
index 6a3920e..d2e4ea1 100644
--- a/tests/spawn_test.cpp
+++ b/tests/spawn_test.cpp
@@ -20,6 +20,7 @@
#include <fcntl.h>
#include <gtest/gtest.h>
+#include "ScopedSignalHandler.h"
#include "utils.h"
#include <android-base/file.h>
@@ -386,3 +387,41 @@
ASSERT_EQ(0, posix_spawnattr_destroy(&sa));
}
+
+TEST(spawn, signal_stress) {
+ // Ensure that posix_spawn doesn't restore the caller's signal mask in the
+ // child without first defaulting any caught signals (http://b/68707996).
+ static pid_t parent = getpid();
+
+ pid_t pid = fork();
+ ASSERT_NE(-1, pid);
+
+ if (pid == 0) {
+ for (size_t i = 0; i < 1024; ++i) {
+ kill(0, SIGWINCH);
+ usleep(10);
+ }
+ return;
+ }
+
+ // We test both with and without attributes, because they used to be
+ // different codepaths. We also test with an empty `sigdefault` set.
+ posix_spawnattr_t attr1;
+ posix_spawnattr_init(&attr1);
+
+ sigset_t empty_mask = {};
+ posix_spawnattr_t attr2;
+ posix_spawnattr_init(&attr2);
+ posix_spawnattr_setflags(&attr2, POSIX_SPAWN_SETSIGDEF);
+ posix_spawnattr_setsigdefault(&attr2, &empty_mask);
+
+ posix_spawnattr_t* attrs[] = { nullptr, &attr1, &attr2 };
+
+ ScopedSignalHandler ssh(SIGWINCH, [](int) { ASSERT_EQ(getpid(), parent); });
+
+ ExecTestHelper eth;
+ eth.SetArgs({"true", nullptr});
+ for (size_t i = 0; i < 128; ++i) {
+ posix_spawn(nullptr, "true", nullptr, attrs[i % 3], eth.GetArgs(), nullptr);
+ }
+}