signals: proper support for sigaltstack() and SA_ONSTACK sigaction flag

When an application gets a SIGSEGV because it overflowed the thread's
stack, it is pointless to try to run a signal handler on the same stack
which has no room to run on. This is why Linux provides a sigaltstack()
function which allows the user to specify a (per-thread) stack for running
signal handlers, and a SA_ONSTACK flag for sigaction() to ask to use the
alternative stack for a specific handler.

So far these features were stubbed, and this patch provides a complete
implementation, and also a test - which overflows the stack and tries
to recover with a signal handler (without SA_ONSTACK properly working,
the test crashes when it tries to run the signal handler).

Fixes #809.

Proper support of sigaltstack() is important for golang (see issue #522).

Signed-off-by: Nadav Har'El <[email protected]>
Message-Id: <[email protected]>

arch/x64/signal.cc

@@ -26,8 +26,24 @@ void build_signal_frame(exception_frame* ef,
                         const siginfo_t& si,
                         const struct sigaction& sa)
 {
-    void* rsp = reinterpret_cast<void*>(ef->rsp);
-    rsp -= 128;                 // skip red zone
+    // If an alternative signal stack was defined for this thread with
+    // sigaltstack() and the SA_ONSTACK flag was specified, we should run
+    // the signal handler on that stack. Otherwise, we need to run further
+    // down the same stack the thread was using when it received the signal:
+    void *rsp = nullptr;
+    if (sa.sa_flags & SA_ONSTACK) {
+        stack_t sigstack;
+        sigaltstack(nullptr, &sigstack);
+        if (!(sigstack.ss_flags & SS_DISABLE)) {
+            // ss_sp points to the beginning of the stack region, but x86
+            // stacks grow downward, from the end of the region
+            rsp = sigstack.ss_sp + sigstack.ss_size;
+        }
+    }
+    if (!rsp) {
+        rsp = reinterpret_cast<void*>(ef->rsp);
+        rsp -= 128;                 // skip red zone
+    }
     rsp -= sizeof(signal_frame);
     // the Linux x86_64 calling conventions want 16-byte aligned rsp.
     // signal_frame may want even stricter alignment (but probably won't).

libc/signal.cc

@@ -584,9 +584,48 @@ extern "C" int getitimer(int which, struct itimerval *curr_value)
     }
 }
 
+// A per-thread stack set with sigaltstack() to be used by
+// build_signal_frame() when handling synchronous signals, most
+// importantly SIGSEGV (asynchronous signals run in a new thread in OSv,
+// so this alternative stack is irrelevant).  signal_stack_begin is the
+// beginning of stack region, and signal_stack_size is the size of this
+// region (we need to know both when stacks grow down).
+// The stack region is not guaranteed to be specially aligned, so when
+// build_signal_frame() uses it it might need to further snipped this region.
+// If signal_stack == nullptr, there is no alternative signal stack for
+// this thread.
+static __thread void* signal_stack_begin;
+static __thread size_t signal_stack_size;
+
 int sigaltstack(const stack_t *ss, stack_t *oss)
 {
-    WARN_STUBBED();
+    if (oss) {
+        if (signal_stack_begin) {
+            // FIXME: we are supposed to set SS_ONSTACK if a signal
+            // handler is currently running on this stack.
+            oss->ss_flags = 0;
+            oss->ss_sp = signal_stack_begin;
+            oss->ss_size = signal_stack_size;
+        } else {
+            oss->ss_flags = SS_DISABLE;
+        }
+    }
+    if (ss == nullptr) {
+        // sigaltstack may be used with ss==nullptr for querying the
+        // current state without changing it.
+        return 0;
+    }
+    // FIXME: we're are supposed to check if a signal handler is currently
+    // running on this stack, and if it is forbid changing it (EPERM).
+    if (ss->ss_flags & SS_DISABLE) {
+        signal_stack_begin = nullptr;
+    } else if (ss->ss_flags != 0) {
+        errno = EINVAL;
+        return -1;
+    } else {
+        signal_stack_begin = ss->ss_sp;
+        signal_stack_size = ss->ss_size;
+    }
     return 0;
 }
 

modules/tests/Makefile

@@ -85,7 +85,8 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so tst-bsd-evh.so \
 	payload-merge-env.so misc-execve.so misc-execve-payload.so misc-mutex2.so \
 	tst-pthread-setcancelstate.so tst-syscall.so tst-pin.so tst-run.so \
 	tst-ifaddrs.so tst-pthread-affinity-inherit.so tst-sem-timed-wait.so \
-	tst-ttyname.so tst-pthread-barrier.so tst-feexcept.so tst-math.so
+	tst-ttyname.so tst-pthread-barrier.so tst-feexcept.so tst-math.so \
+	tst-sigaltstack.so
 
 #	libstatic-thread-variable.so tst-static-thread-variable.so \
 

tests/tst-sigaltstack.cc

@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2017 ScyllaDB, Ltd.
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+// This test works on both Linux and OSv.
+// To compile on Linux, use: c++ -std=c++11 tests/tst-sigaltstack.cc
+
+#include <signal.h>
+#include <assert.h>
+#include <setjmp.h>
+#include <sys/mman.h>
+
+#include <iostream>
+
+static int tests = 0, fails = 0;
+
+#define expect(actual, expected) do_expect(actual, expected, #actual, #expected, __FILE__, __LINE__)
+template<typename T>
+bool do_expect(T actual, T expected, const char *actuals, const char *expecteds, const char *file, int line)
+{
+    ++tests;
+    if (actual != expected) {
+        fails++;
+        std::cout << "FAIL: " << file << ":" << line << ": For " << actuals <<
+                ", expected " << expecteds << ", saw " << actual << ".\n";
+        return false;
+    }
+    return true;
+}
+
+#define expect_errno(call, experrno) ( \
+        do_expect(call, -1, #call, "-1", __FILE__, __LINE__) && \
+        do_expect(errno, experrno, #call " errno",  #experrno, __FILE__, __LINE__) )
+
+thread_local sigjmp_buf env;
+template<typename Func>
+bool sig_check(Func f, int sig) {
+    if (sigsetjmp(env, 1)) {
+        // got the signal (and automatically restored handler)
+        return true;
+    }
+    struct sigaction sa;
+    // Note: we use SA_ONSTACK to have this run on the sigaltstack() stack
+    sa.sa_flags = SA_RESETHAND | SA_ONSTACK;
+    sigemptyset(&sa.sa_mask);
+    sa.sa_handler = [] (int) {
+        siglongjmp(env, 1);
+    };
+    assert(sigaction(sig, &sa, NULL) == 0);
+    f();
+    sa.sa_handler = SIG_DFL;
+    assert(sigaction(sig, &sa, NULL) == 0);
+    return false;
+}
+
+// endless_recursive() recurses ad infinitum, so it will finish the entire
+// stack and finally reach an unmapped area and cause a SIGSEGV. When it
+// does, a SIGSEGV handler will not be able to run on the normal thread stack
+// (which ran out), and only if sigaltstack() is properly supported, will the
+// signal handler be able to run.
+int endless_recursive() {
+    return endless_recursive() + endless_recursive();
+}
+
+int main(int argc, char **argv)
+{
+    // setup the per-thread sigaltstack():
+    stack_t sigstack;
+    sigstack.ss_size = SIGSTKSZ;
+    sigstack.ss_sp = malloc(sigstack.ss_size);
+    sigstack.ss_flags = 0;
+
+    // Enable the sigaltstack. Being able to catch SIGSEGV after
+    // endless_recursive() - as we do below - will not work without it
+    stack_t oldstack {};
+    expect(sigaltstack(&sigstack, &oldstack), 0);
+    // We didn't have a sigaltstack previously, so oldstack is just disabled
+    expect(oldstack.ss_flags, (int)SS_DISABLE);
+
+    expect(sig_check(endless_recursive, SIGSEGV), true);
+
+    // Check that disabling a stack works
+    sigstack.ss_flags = SS_DISABLE;
+    oldstack = {};
+    expect(sigaltstack(&sigstack, &oldstack), 0);
+    // The previous stack was the one we had
+    expect(oldstack.ss_sp, sigstack.ss_sp);
+    expect(oldstack.ss_size, sigstack.ss_size);
+
+    free(sigstack.ss_sp);
+
+    std::cout << "SUMMARY: " << tests << " tests, " << fails << " failures\n";
+    return fails == 0 ? 0 : 1;
+}