[master] 17f4c18 Add a basic stack overflow detection heuristic

Nils Goroll nils.goroll at uplex.de
Tue Apr 10 14:18:10 UTC 2018


commit 17f4c18ee8d1f25d77c41168da7274e7331815b7
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Tue Apr 10 16:12:24 2018 +0200

    Add a basic stack overflow detection heuristic
    
    Yes, I know, this is far from perfect, but it does not require
    any changes outside the signal handler and will hopefully simplify
    error report handling significantly.
    
    Ref #2643

diff --git a/bin/varnishd/cache/cache_main.c b/bin/varnishd/cache/cache_main.c
index ad7e312..fe8cdb3 100644
--- a/bin/varnishd/cache/cache_main.c
+++ b/bin/varnishd/cache/cache_main.c
@@ -235,17 +235,36 @@ child_signal_handler(int s, siginfo_t *si, void *c)
 {
 	char buf[1024];
 	struct sigaction sa;
+	struct req *req;
+	const char *a, *p, *info = NULL;
 
 	(void)c;
-
 	/* Don't come back */
 	memset(&sa, 0, sizeof sa);
 	sa.sa_handler = SIG_DFL;
 	(void)sigaction(SIGSEGV, &sa, NULL);
 	(void)sigaction(SIGABRT, &sa, NULL);
 
-	bprintf(buf, "Signal %d (%s) received at %p si_code %d",
-		s, strsignal(s), si->si_addr, si->si_code);
+	while (s == SIGSEGV) {
+		req = THR_GetRequest();
+		if (req == NULL || req->wrk == NULL)
+			break;
+		a = TRUST_ME(si->si_addr);
+		p = TRUST_ME(req->wrk);
+		p += sizeof *req->wrk;
+		// rough safe estimate - top of stack
+		if (a > p + cache_param->wthread_stacksize)
+			break;
+		if (a < p - 2 * cache_param->wthread_stacksize)
+			break;
+		info = "\nTHIS PROBABLY IS A STACK OVERFLOW - "
+			"check thread_pool_stack parameter";
+		break;
+	}
+	bprintf(buf, "Signal %d (%s) received at %p si_code %d%s",
+		s, strsignal(s), si->si_addr, si->si_code,
+		info ? info : "");
+
 	VAS_Fail(__func__,
 		 __FILE__,
 		 __LINE__,
diff --git a/bin/varnishd/cache/cache_wrk.c b/bin/varnishd/cache/cache_wrk.c
index 2a9c75d..2c0ad15 100644
--- a/bin/varnishd/cache/cache_wrk.c
+++ b/bin/varnishd/cache/cache_wrk.c
@@ -96,6 +96,7 @@ WRK_BgThread(pthread_t *thr, const char *name, bgthread_t *func, void *priv)
 static void
 WRK_Thread(struct pool *qp, size_t stacksize, unsigned thread_workspace)
 {
+	// child_signal_handler stack overflow check uses struct worker addr
 	struct worker *w, ww;
 	struct VSC_main ds;
 	unsigned char ws[thread_workspace];
diff --git a/bin/varnishtest/tests/c00057.vtc b/bin/varnishtest/tests/c00057.vtc
index 4b05a8f..c70158e 100644
--- a/bin/varnishtest/tests/c00057.vtc
+++ b/bin/varnishtest/tests/c00057.vtc
@@ -39,6 +39,8 @@ client c1 {
 	expect_close
 } -run
 
+varnish v1 -cliexpect "STACK OVERFLOW" "panic.show"
+
 varnish v1 -cliok "panic.clear"
 
 # Also check without the handler
@@ -51,3 +53,31 @@ client c1 {
 } -run
 
 varnish v1 -expectexit 0x20
+
+####################
+
+varnish v2 \
+	-arg "-p feature=+no_coredump" \
+	-arg "-p vcc_allow_inline_c=true" \
+	-vcl+backend {
+
+	C{
+	#include <stdio.h>
+	}C
+
+	sub vcl_recv { C{
+	    int *i = (void *)VRT_GetHdr;
+	    *i = 42;
+	}C }
+} -start
+
+client c2 -connect ${v2_sock} {
+	txreq
+	expect_close
+} -run
+
+varnish v2 -cliexpect "Segmentation fault" "panic.show"
+
+varnish v2 -cliok "panic.clear"
+
+varnish v2 -expectexit 0x20


More information about the varnish-commit mailing list