The getcontext() from the __fillcontextx() call in the

check_deferred_signal() returns twice, since handle_signal() emulates
the return from the normal signal handler by sigreturn(2)ing the
passed context.  Second return is performed on the destroyed stack
frame, because __fillcontextx() has already returned.  This causes
undefined and bad behaviour, usually the victim thread gets SIGSEGV.

Avoid nested frame and the need to return from it by doing direct call
to getcontext() in the check_deferred_signal() and using a new private
libc helper __fillcontextx2() to complement the context with the
extended CPU state if the deferred signal is still present.

The __fillcontextx() is now unused, but is kept to allow older
libthr.so to be used with the new libc.

Mark __fillcontextx() as returning twice [1].

Reported by:	pgj
Pointy hat to:	kib
Discussed with:	dim
Tested by:	pgj, dim
Suggested by:	jilles [1]
MFC after:	1 week

11 files changed, 78 insertions, 9 deletions

diff --git a/lib/libc/amd64/gen/getcontextx.c b/lib/libc/amd64/gen/getcontextx.c
index 7412560..749bfbd 100644
--- a/lib/libc/amd64/gen/getcontextx.c
+++ b/lib/libc/amd64/gen/getcontextx.c
@@ -57,14 +57,12 @@ __getcontextx_size(void)
 }
 
 int
-__fillcontextx(char *ctx)
+__fillcontextx2(char *ctx)
 {
 	struct amd64_get_xfpustate xfpu;
 	ucontext_t *ucp;
 
 	ucp = (ucontext_t *)ctx;
-	if (getcontext(ucp) == -1)
-		return (-1);
 	if (xstate_sz != 0) {
 		xfpu.addr = (char *)(ucp + 1);
 		xfpu.len = xstate_sz;
@@ -80,6 +78,18 @@ __fillcontextx(char *ctx)
 	return (0);
 }
 
+int
+__fillcontextx(char *ctx)
+{
+	ucontext_t *ucp;
+
+	ucp = (ucontext_t *)ctx;
+	if (getcontext(ucp) == -1)
+		return (-1);
+	__fillcontextx2(ctx);
+	return (0);
+}
+
 __weak_reference(__getcontextx, getcontextx);
 
 ucontext_t *
diff --git a/lib/libc/arm/gen/getcontextx.c b/lib/libc/arm/gen/getcontextx.c
index 307978a..54f8513 100644
--- a/lib/libc/arm/gen/getcontextx.c
+++ b/lib/libc/arm/gen/getcontextx.c
@@ -40,6 +40,13 @@ __getcontextx_size(void)
 }
 
 int
+__fillcontextx2(char *ctx)
+{
+
+	return (0);
+}
+
+int
 __fillcontextx(char *ctx)
 {
 	ucontext_t *ucp;
diff --git a/lib/libc/gen/Symbol.map b/lib/libc/gen/Symbol.map
index 2438951..9e6b38b 100644
--- a/lib/libc/gen/Symbol.map
+++ b/lib/libc/gen/Symbol.map
@@ -529,5 +529,6 @@ FBSDprivate_1.0 {
 	__elf_aux_vector;
 	__pthread_map_stacks_exec;
 	__fillcontextx;
+	__fillcontextx2;
 	__getcontextx_size;
 };
diff --git a/lib/libc/i386/gen/getcontextx.c b/lib/libc/i386/gen/getcontextx.c
index 2d3b562..3c6dd33 100644
--- a/lib/libc/i386/gen/getcontextx.c
+++ b/lib/libc/i386/gen/getcontextx.c
@@ -89,14 +89,12 @@ __getcontextx_size(void)
 }
 
 int
-__fillcontextx(char *ctx)
+__fillcontextx2(char *ctx)
 {
 	struct i386_get_xfpustate xfpu;
 	ucontext_t *ucp;
 
 	ucp = (ucontext_t *)ctx;
-	if (getcontext(ucp) == -1)
-		return (-1);
 	if (xstate_sz != 0) {
 		xfpu.addr = (char *)(ucp + 1);
 		xfpu.len = xstate_sz;
@@ -112,6 +110,18 @@ __fillcontextx(char *ctx)
 	return (0);
 }
 
+int
+__fillcontextx(char *ctx)
+{
+	ucontext_t *ucp;
+
+	ucp = (ucontext_t *)ctx;
+	if (getcontext(ucp) == -1)
+		return (-1);
+	__fillcontextx2(ctx);
+	return (0);
+}
+
 __weak_reference(__getcontextx, getcontextx);
 
 ucontext_t *
diff --git a/lib/libc/ia64/gen/getcontextx.c b/lib/libc/ia64/gen/getcontextx.c
index 307978a..54f8513 100644
--- a/lib/libc/ia64/gen/getcontextx.c
+++ b/lib/libc/ia64/gen/getcontextx.c
@@ -40,6 +40,13 @@ __getcontextx_size(void)
 }
 
 int
+__fillcontextx2(char *ctx)
+{
+
+	return (0);
+}
+
+int
 __fillcontextx(char *ctx)
 {
 	ucontext_t *ucp;
diff --git a/lib/libc/mips/gen/getcontextx.c b/lib/libc/mips/gen/getcontextx.c
index 307978a..54f8513 100644
--- a/lib/libc/mips/gen/getcontextx.c
+++ b/lib/libc/mips/gen/getcontextx.c
@@ -40,6 +40,13 @@ __getcontextx_size(void)
 }
 
 int
+__fillcontextx2(char *ctx)
+{
+
+	return (0);
+}
+
+int
 __fillcontextx(char *ctx)
 {
 	ucontext_t *ucp;
diff --git a/lib/libc/powerpc/gen/getcontextx.c b/lib/libc/powerpc/gen/getcontextx.c
index 307978a..54f8513 100644
--- a/lib/libc/powerpc/gen/getcontextx.c
+++ b/lib/libc/powerpc/gen/getcontextx.c
@@ -40,6 +40,13 @@ __getcontextx_size(void)
 }
 
 int
+__fillcontextx2(char *ctx)
+{
+
+	return (0);
+}
+
+int
 __fillcontextx(char *ctx)
 {
 	ucontext_t *ucp;
diff --git a/lib/libc/powerpc64/gen/getcontextx.c b/lib/libc/powerpc64/gen/getcontextx.c
index 307978a..54f8513 100644
--- a/lib/libc/powerpc64/gen/getcontextx.c
+++ b/lib/libc/powerpc64/gen/getcontextx.c
@@ -40,6 +40,13 @@ __getcontextx_size(void)
 }
 
 int
+__fillcontextx2(char *ctx)
+{
+
+	return (0);
+}
+
+int
 __fillcontextx(char *ctx)
 {
 	ucontext_t *ucp;
diff --git a/lib/libc/sparc64/gen/getcontextx.c b/lib/libc/sparc64/gen/getcontextx.c
index 307978a..54f8513 100644
--- a/lib/libc/sparc64/gen/getcontextx.c
+++ b/lib/libc/sparc64/gen/getcontextx.c
@@ -40,6 +40,13 @@ __getcontextx_size(void)
 }
 
 int
+__fillcontextx2(char *ctx)
+{
+
+	return (0);
+}
+
+int
 __fillcontextx(char *ctx)
 {
 	ucontext_t *ucp;
diff --git a/lib/libthr/thread/thr_sig.c b/lib/libthr/thread/thr_sig.c
index 86b3a9b..4dbb252 100644
--- a/lib/libthr/thread/thr_sig.c
+++ b/lib/libthr/thread/thr_sig.c
@@ -323,8 +323,13 @@ check_deferred_signal(struct pthread *curthread)
 		return;
 
 #if defined(__amd64__) || defined(__i386__)
-	uc = alloca(__getcontextx_size());
-	__fillcontextx((char *)uc);
+	int uc_len;
+	uc_len = __getcontextx_size();
+	uc = alloca(uc_len);
+	getcontext(uc);
+	if (curthread->deferred_siginfo.si_signo == 0)
+		return;
+	__fillcontextx2((char *)uc);
 #else
 	ucontext_t ucv;
 	uc = &ucv;
diff --git a/sys/sys/ucontext.h b/sys/sys/ucontext.h
index 1e504c9..f69c8d8 100644
--- a/sys/sys/ucontext.h
+++ b/sys/sys/ucontext.h
@@ -80,7 +80,8 @@ int	swapcontext(ucontext_t *, const ucontext_t *);
 
 #if __BSD_VISIBLE
 int __getcontextx_size(void);
-int __fillcontextx(char *ctx);
+int __fillcontextx(char *ctx) __returns_twice;
+int __fillcontextx2(char *ctx);
 #endif
 
 __END_DECLS