[master] 438cc27 use an alternative stack for SIGSEGV and test for stack overflow

Nils Goroll nils.goroll at uplex.de
Mon Aug 28 14:05:11 CEST 2017


commit 438cc27eccd2fccdff6d9410c69f695443fbb806
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Wed Aug 23 08:08:59 2017 +0200

    use an alternative stack for SIGSEGV and test for stack overflow
    
    Previously, we could run out of stack handling stack overflows, leaving
    users with unspecific SIGSEGV crashes and no panic message.
    
    By providing a single alternative stack exclusively for SIGSEGV handling
    where sigaltstack() is available, we increase chances for our signal handler
    to finish successfully.
    
    In particular, this will make it easier to diagnose stack overflows by
    comparing the failing address with the stack info from the panic output.
    
    This could be further improved by giving advise to increase thread_pool_stack
    if si_addr is near the stack boundaries.
    
    c00057.vtc now triggers a stack overflow instead of raising a SIGSEGV.
    
    Merges #2396

diff --git a/bin/varnishd/mgt/mgt_child.c b/bin/varnishd/mgt/mgt_child.c
index d3dcbf0..47bd905 100644
--- a/bin/varnishd/mgt/mgt_child.c
+++ b/bin/varnishd/mgt/mgt_child.c
@@ -352,9 +352,23 @@ mgt_launch_child(struct cli *cli)
 			memset(&sa, 0, sizeof sa);
 			sa.sa_sigaction = child_signal_handler;
 			sa.sa_flags = SA_SIGINFO;
-			(void)sigaction(SIGSEGV, &sa, NULL);
 			(void)sigaction(SIGBUS, &sa, NULL);
 			(void)sigaction(SIGABRT, &sa, NULL);
+
+#ifdef HAVE_SIGALTSTACK
+			stack_t ss;
+			size_t sz = SIGSTKSZ + 4096;
+			if (sz < mgt_param.wthread_stacksize)
+				sz = mgt_param.wthread_stacksize;
+			ss.ss_sp = malloc(sz);
+			AN(ss.ss_sp);
+			ss.ss_size = sz;
+			ss.ss_flags = 0;
+			AZ(sigaltstack(&ss, NULL));
+			sa.sa_flags |= SA_ONSTACK;
+#endif
+			(void)sigaction(SIGSEGV, &sa, NULL);
+
 		}
 		(void)signal(SIGINT, SIG_DFL);
 		(void)signal(SIGTERM, SIG_DFL);
diff --git a/bin/varnishtest/tests/c00057.vtc b/bin/varnishtest/tests/c00057.vtc
index 6ad74aa..c7ee3c6 100644
--- a/bin/varnishtest/tests/c00057.vtc
+++ b/bin/varnishtest/tests/c00057.vtc
@@ -5,13 +5,32 @@ server s1 {
 	txresp
 } -start
 
-varnish v1 -cliok "param.set vcc_allow_inline_c true"
-varnish v1 -vcl+backend {
+varnish v1 \
+	-arg "-p vcc_allow_inline_c=true" \
+	-arg "-p thread_pool_stack=48k" \
+	-vcl+backend {
 	C{
 #include <signal.h>
 #include <unistd.h>
+#include <stdio.h>
+
+static void _accessor(volatile char *p) {
+     p[0] = 'V'; p[1] = '\0';
+     fprintf(stderr, "%p %s\n", p, p);
+}
+void (*accessor)(volatile char *p) = _accessor;
+
 }C
-	sub vcl_recv { C{ raise(SIGSEGV); sleep(2); }C }
+	sub vcl_recv { C{
+	    int i;
+	    volatile char overflow[48*1024];
+
+	    /* for downwards stack, take care to hit a single guard page */
+	    for (i = 47*1024; i >= 0; i -= 1024)
+		accessor(overflow + i);
+	    /* NOTREACHED */
+	    sleep(2);
+	}C }
 } -start
 
 client c1 {
diff --git a/configure.ac b/configure.ac
index 0f714d6..4c58ea1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -199,6 +199,7 @@ AC_CHECK_FUNCS([nanosleep])
 AC_CHECK_FUNCS([setppriv])
 AC_CHECK_FUNCS([fallocate])
 AC_CHECK_FUNCS([closefrom])
+AC_CHECK_FUNCS([sigaltstack])
 
 save_LIBS="${LIBS}"
 LIBS="${PTHREAD_LIBS}"



More information about the varnish-commit mailing list