[PATCH v6(resend2) 0/4] make sigaltstack compatible with swapcontext()

[PATCH v6(resend2) 0/4] make sigaltstack compatible with swapcontext()

[Stas Sergeev]

The following patches make it possible to use swapcontext()
in a sighandler that works on sigaltstack.
The approach is inspired by Andy Lutomirski's suggestion that
sigaltstack should disarm itself after saving into uc_stack:
https://lkml.org/lkml/2016/2/1/594

I add the SS_AUTODISARM flag that does exactly that.
On sighandler exit, the sigaltstack is restored from uc_stack.
Another possible name could be SS_ONESHOT, but, since it gets
always re-enabled, I choose SS_AUTODISARM.

Changes since previous resend:
- Added missing CCs, thanks for the hint Ingo

Change since v5:
- Fix description of patch 4/4

Change since v4:
- Implement this Andy Lutomirski's suggestion:
https://lkml.org/lkml/2016/3/6/158
that allows the run-time probing of the existence of the
flags added in the future.

[PATCH 1/4] [Cleanup] x86: signal: unify the sigaltstack check with
	A clean-up patch that unifies x86's sigaltstack handling with
	other arches.
[PATCH 2/4] sigaltstack: preparations for adding new SS_xxx flags
	Andy's suggested changes
[PATCH 3/4] sigaltstack: implement SS_AUTODISARM flag
	This patch implements SS_AUTODISARM flag
[PATCH 4/4] selftests: Add test for
	This patch adds the selftest code for new functionality

CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Shuah Khan <shuahkh@osg.samsung.com>
CC: Ingo Molnar <mingo@redhat.com>

Diffstat:
 arch/x86/kernel/signal.c                     |   23 +--
 include/linux/sched.h                        |    8 +
 include/linux/signal.h                       |    4 
 include/uapi/linux/signal.h                  |    7 +
 kernel/fork.c                                |    2 
 kernel/signal.c                              |   26 ++--
 tools/testing/selftests/Makefile             |    1 
 tools/testing/selftests/sigaltstack/Makefile |    8 +
 tools/testing/selftests/sigaltstack/sas.c    |  156 +++++++++++++++++++++++++++
 9 files changed, 208 insertions(+), 27 deletions(-)

[PATCH 1/4] [Cleanup] x86: signal: unify the sigaltstack check with other arches

[Stas Sergeev]

Currently x86's get_sigframe() checks for "current->sas_ss_size"
to determine whether there is a need to switch to sigaltstack.
The common practice used by all other arches is to check for
sas_ss_flags(sp) == 0

This patch makes the code consistent with other arches.
The slight complexity of the patch is added by the optimization on
!sigstack check that was requested by Andy Lutomirski: sas_ss_flags(sp)==0
already implies that we are not on a sigstack, so the code is shuffled
to avoid the duplicate checking.

This patch have no any user-visible impact. It is purely a cleanup.

CC: linux-kernel@vger.kernel.org
CC: Andy Lutomirski <luto@amacapital.net>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: Borislav Petkov <bp@suse.de>
CC: Brian Gerst <brgerst@gmail.com>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Richard Weinberger <richard@nod.at>
CC: linux-api@vger.kernel.org
CC: Shuah Khan <shuahkh@osg.samsung.com>

Signed-off-by: Stas Sergeev <stsp@list.ru>
---
 arch/x86/kernel/signal.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index cb6282c..285183b 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -213,18 +213,17 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
 	if (config_enabled(CONFIG_X86_64))
 		sp -= 128;
 
-	if (!onsigstack) {
-		/* This is the X/Open sanctioned signal stack switching.  */
-		if (ka->sa.sa_flags & SA_ONSTACK) {
-			if (current->sas_ss_size)
-				sp = current->sas_ss_sp + current->sas_ss_size;
-		} else if (config_enabled(CONFIG_X86_32) &&
-			   (regs->ss & 0xffff) != __USER_DS &&
-			   !(ka->sa.sa_flags & SA_RESTORER) &&
-			   ka->sa.sa_restorer) {
-				/* This is the legacy signal stack switching. */
-				sp = (unsigned long) ka->sa.sa_restorer;
-		}
+	/* This is the X/Open sanctioned signal stack switching.  */
+	if (ka->sa.sa_flags & SA_ONSTACK) {
+		if (sas_ss_flags(sp) == 0)
+			sp = current->sas_ss_sp + current->sas_ss_size;
+	} else if (config_enabled(CONFIG_X86_32) &&
+		   !onsigstack &&
+		   (regs->ss & 0xffff) != __USER_DS &&
+		   !(ka->sa.sa_flags & SA_RESTORER) &&
+		   ka->sa.sa_restorer) {
+		/* This is the legacy signal stack switching. */
+		sp = (unsigned long) ka->sa.sa_restorer;
 	}
 
 	if (fpu->fpstate_active) {
-- 
2.7.2

[PATCH 2/4] sigaltstack: preparations for adding new SS_xxx flags

[Stas Sergeev]

This patch adds SS_FLAG_BITS - the mask that splits sigaltstack
mode values and bit-flags. Since there is no bit-flags yet, the
mask is defined to 0. The flags are added by subsequent patches.
With every new flag, the mask should have the appropriate bit cleared.

This makes sure if some flag is tried on a kernel that doesn't
support it, the EINVAL error will be returned, because such a
flag will be treated as an invalid mode rather than the bit-flag.
That way the existence of the particular features can be probed
at run-time.

This change was suggested by Andy Lutomirski:
https://lkml.org/lkml/2016/3/6/158

Signed-off-by: Stas Sergeev <stsp@list.ru>

CC: Andrew Morton <akpm@linux-foundation.org>
CC: Oleg Nesterov <oleg@redhat.com>
CC: "Peter Zijlstra (Intel)" <peterz@infradead.org>
CC: "Amanieu d'Antras" <amanieu@gmail.com>
CC: Michal Hocko <mhocko@suse.com>
CC: Richard Weinberger <richard@nod.at>
CC: Vladimir Davydov <vdavydov@parallels.com>
CC: Sasha Levin <sasha.levin@oracle.com>
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: Ingo Molnar <mingo@kernel.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Shuah Khan <shuahkh@osg.samsung.com>
---
 include/uapi/linux/signal.h |  3 +++
 kernel/signal.c             | 16 ++++++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index e1bd50c2..7c73165 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -7,4 +7,7 @@
 #define SS_ONSTACK	1
 #define SS_DISABLE	2
 
+/* mask for all SS_xxx flags */
+#define SS_FLAG_BITS	0
+
 #endif /* _UAPI_LINUX_SIGNAL_H */
diff --git a/kernel/signal.c b/kernel/signal.c
index 0508544..9a24bc3 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3100,7 +3100,8 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 	if (uss) {
 		void __user *ss_sp;
 		size_t ss_size;
-		int ss_flags;
+		unsigned ss_flags;
+		int ss_mode;
 
 		error = -EFAULT;
 		if (!access_ok(VERIFY_READ, uss, sizeof(*uss)))
@@ -3115,18 +3116,13 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 		if (on_sig_stack(sp))
 			goto out;
 
+		ss_mode = ss_flags & ~SS_FLAG_BITS;
 		error = -EINVAL;
-		/*
-		 * Note - this code used to test ss_flags incorrectly:
-		 *  	  old code may have been written using ss_flags==0
-		 *	  to mean ss_flags==SS_ONSTACK (as this was the only
-		 *	  way that worked) - this fix preserves that older
-		 *	  mechanism.
-		 */
-		if (ss_flags != SS_DISABLE && ss_flags != SS_ONSTACK && ss_flags != 0)
+		if (ss_mode != SS_DISABLE && ss_mode != SS_ONSTACK &&
+				ss_mode != 0)
 			goto out;
 
-		if (ss_flags == SS_DISABLE) {
+		if (ss_mode == SS_DISABLE) {
 			ss_size = 0;
 			ss_sp = NULL;
 		} else {
-- 
2.7.2

[PATCH 3/4] sigaltstack: implement SS_AUTODISARM flag

[Stas Sergeev]

This patch implements the SS_AUTODISARM flag that can be ORed with
SS_ONSTACK when forming ss_flags.
When this flag is set, sigaltstack will be disabled when entering
the signal handler; more precisely, after saving sas to uc_stack.
When leaving the signal handler, the sigaltstack is restored by
uc_stack.
When this flag is used, it is safe to switch from sighandler with
swapcontext(). Without this flag, the subsequent signal will corrupt
the state of the switched-away sighandler.

To detect the support of this functionality, one can do
err = sigaltstack(SS_DISABLE | SS_AUTODISARM);
if (err && errno == EINVAL) unsupported();

Signed-off-by: Stas Sergeev <stsp@list.ru>

CC: Ingo Molnar <mingo@redhat.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Richard Weinberger <richard@nod.at>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Tejun Heo <tj@kernel.org>
CC: Heinrich Schuchardt <xypron.glpk@gmx.de>
CC: Jason Low <jason.low2@hp.com>
CC: Andrea Arcangeli <aarcange@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
CC: Josh Triplett <josh@joshtriplett.org>
CC: "Eric W. Biederman" <ebiederm@xmission.com>
CC: Aleksa Sarai <cyphar@cyphar.com>
CC: "Amanieu d'Antras" <amanieu@gmail.com>
CC: Paul Moore <pmoore@redhat.com>
CC: Sasha Levin <sasha.levin@oracle.com>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: Vladimir Davydov <vdavydov@parallels.com>
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: Andy Lutomirski <luto@amacapital.net>
CC: Shuah Khan <shuahkh@osg.samsung.com>
---
 include/linux/sched.h       |  8 ++++++++
 include/linux/signal.h      |  4 +++-
 include/uapi/linux/signal.h |  4 +++-
 kernel/fork.c               |  2 +-
 kernel/signal.c             | 10 ++++++++--
 5 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a10494a..26201cd 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1587,6 +1587,7 @@ struct task_struct {
 
 	unsigned long sas_ss_sp;
 	size_t sas_ss_size;
+	unsigned sas_ss_flags;
 
 	struct callback_head *task_works;
 
@@ -2573,6 +2574,13 @@ static inline int sas_ss_flags(unsigned long sp)
 	return on_sig_stack(sp) ? SS_ONSTACK : 0;
 }
 
+static inline void sas_ss_reset(struct task_struct *p)
+{
+	p->sas_ss_sp = 0;
+	p->sas_ss_size = 0;
+	p->sas_ss_flags = SS_DISABLE;
+}
+
 static inline unsigned long sigsp(unsigned long sp, struct ksignal *ksig)
 {
 	if (unlikely((ksig->ka.sa.sa_flags & SA_ONSTACK)) && ! sas_ss_flags(sp))
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 92557bb..3fbe814 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -432,8 +432,10 @@ int __save_altstack(stack_t __user *, unsigned long);
 	stack_t __user *__uss = uss; \
 	struct task_struct *t = current; \
 	put_user_ex((void __user *)t->sas_ss_sp, &__uss->ss_sp); \
-	put_user_ex(sas_ss_flags(sp), &__uss->ss_flags); \
+	put_user_ex(t->sas_ss_flags, &__uss->ss_flags); \
 	put_user_ex(t->sas_ss_size, &__uss->ss_size); \
+	if (t->sas_ss_flags & SS_AUTODISARM) \
+		sas_ss_reset(t); \
 } while (0);
 
 #ifdef CONFIG_PROC_FS
diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index 7c73165..7388260 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -7,7 +7,9 @@
 #define SS_ONSTACK	1
 #define SS_DISABLE	2
 
+/* bit-flags */
+#define SS_AUTODISARM	(1 << 4)	/* disable sas during sighandling */
 /* mask for all SS_xxx flags */
-#define SS_FLAG_BITS	0
+#define SS_FLAG_BITS	SS_AUTODISARM
 
 #endif /* _UAPI_LINUX_SIGNAL_H */
diff --git a/kernel/fork.c b/kernel/fork.c
index 2e391c7..68c8716 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1483,7 +1483,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	 * sigaltstack should be cleared when sharing the same VM
 	 */
 	if ((clone_flags & (CLONE_VM|CLONE_VFORK)) == CLONE_VM)
-		p->sas_ss_sp = p->sas_ss_size = 0;
+		sas_ss_reset(p);
 
 	/*
 	 * Syscall tracing and stepping should be turned off in the
diff --git a/kernel/signal.c b/kernel/signal.c
index 9a24bc3..e1096ad 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3133,6 +3133,7 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 
 		current->sas_ss_sp = (unsigned long) ss_sp;
 		current->sas_ss_size = ss_size;
+		current->sas_ss_flags = ss_flags;
 	}
 
 	error = 0;
@@ -3163,9 +3164,14 @@ int restore_altstack(const stack_t __user *uss)
 int __save_altstack(stack_t __user *uss, unsigned long sp)
 {
 	struct task_struct *t = current;
-	return  __put_user((void __user *)t->sas_ss_sp, &uss->ss_sp) |
-		__put_user(sas_ss_flags(sp), &uss->ss_flags) |
+	int err = __put_user((void __user *)t->sas_ss_sp, &uss->ss_sp) |
+		__put_user(t->sas_ss_flags, &uss->ss_flags) |
 		__put_user(t->sas_ss_size, &uss->ss_size);
+	if (err)
+		return err;
+	if (t->sas_ss_flags & SS_AUTODISARM)
+		sas_ss_reset(t);
+	return 0;
 }
 
 #ifdef CONFIG_COMPAT
-- 
2.7.2

[PATCH 4/4] selftests: Add test for sigaltstack(SS_ONSTACK|SS_AUTODISARM)

[Stas Sergeev]

This patch adds the test case for SS_AUTODISARM flag.
The test-case tries to set SS_AUTODISARM flag and checks if
the nested signal corrupts the stack after swapcontext().

CC: Shuah Khan <shuahkh@osg.samsung.com>
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: Andy Lutomirski <luto@amacapital.net>
CC: Ingo Molnar <mingo@kernel.org>
CC: Oleg Nesterov <oleg@redhat.com>

Signed-off-by: Stas Sergeev <stsp@list.ru>
---
 tools/testing/selftests/Makefile             |   1 +
 tools/testing/selftests/sigaltstack/Makefile |   8 ++
 tools/testing/selftests/sigaltstack/sas.c    | 156 +++++++++++++++++++++++++++
 3 files changed, 165 insertions(+)
 create mode 100644 tools/testing/selftests/sigaltstack/Makefile
 create mode 100644 tools/testing/selftests/sigaltstack/sas.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index b04afc3..ff9e5f2 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -19,6 +19,7 @@ TARGETS += powerpc
 TARGETS += pstore
 TARGETS += ptrace
 TARGETS += seccomp
+TARGETS += sigaltstack
 TARGETS += size
 TARGETS += static_keys
 TARGETS += sysctl
diff --git a/tools/testing/selftests/sigaltstack/Makefile b/tools/testing/selftests/sigaltstack/Makefile
new file mode 100644
index 0000000..56af56e
--- /dev/null
+++ b/tools/testing/selftests/sigaltstack/Makefile
@@ -0,0 +1,8 @@
+CFLAGS = -Wall
+BINARIES = sas
+all: $(BINARIES)
+
+include ../lib.mk
+
+clean:
+	rm -rf $(BINARIES)
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
new file mode 100644
index 0000000..57da8bf
--- /dev/null
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -0,0 +1,156 @@
+/*
+ * Stas Sergeev <stsp@users.sourceforge.net>
+ *
+ * test sigaltstack(SS_ONSTACK | SS_AUTODISARM)
+ * If that succeeds, then swapcontext() can be used inside sighandler safely.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+#include <ucontext.h>
+#include <alloca.h>
+#include <string.h>
+#include <assert.h>
+
+#ifndef SS_AUTODISARM
+#define SS_AUTODISARM  (1 << 4)
+#endif
+
+static void *sstack, *ustack;
+static ucontext_t uc, sc;
+static const char *msg = "[OK]\tStack preserved";
+static const char *msg2 = "[FAIL]\tStack corrupted";
+struct stk_data {
+	char msg[128];
+	int flag;
+};
+
+void my_usr1(int sig, siginfo_t *si, void *u)
+{
+	char *aa;
+	int err;
+	stack_t stk;
+	struct stk_data *p;
+
+	register unsigned long sp asm("sp");
+
+	if (sp < (unsigned long)sstack ||
+			sp >= (unsigned long)sstack + SIGSTKSZ) {
+		printf("[FAIL]\tSP is not on sigaltstack\n");
+		exit(EXIT_FAILURE);
+	}
+	/* put some data on stack. other sighandler will try to overwrite it */
+	aa = alloca(1024);
+	assert(aa);
+	p = (struct stk_data *)(aa + 512);
+	strcpy(p->msg, msg);
+	p->flag = 1;
+	printf("[RUN]\tsignal USR1\n");
+	err = sigaltstack(NULL, &stk);
+	if (err) {
+		perror("[FAIL]\tsigaltstack()");
+		exit(EXIT_FAILURE);
+	}
+	if (stk.ss_flags != SS_DISABLE)
+		printf("[FAIL]\tss_flags=%i, should be SS_DISABLE\n",
+				stk.ss_flags);
+	else
+		printf("[OK]\tsigaltstack is disabled in sighandler\n");
+	swapcontext(&sc, &uc);
+	printf("%s\n", p->msg);
+	if (!p->flag) {
+		printf("[RUN]\tAborting\n");
+		exit(EXIT_FAILURE);
+	}
+}
+
+void my_usr2(int sig, siginfo_t *si, void *u)
+{
+	char *aa;
+	struct stk_data *p;
+
+	printf("[RUN]\tsignal USR2\n");
+	aa = alloca(1024);
+	/* dont run valgrind on this */
+	/* try to find the data stored by previous sighandler */
+	p = memmem(aa, 1024, msg, strlen(msg));
+	if (p) {
+		printf("[FAIL]\tsigaltstack re-used\n");
+		/* corrupt the data */
+		strcpy(p->msg, msg2);
+		/* tell other sighandler that his data is corrupted */
+		p->flag = 0;
+	}
+}
+
+static void switch_fn(void)
+{
+	printf("[RUN]\tswitched to user ctx\n");
+	raise(SIGUSR2);
+	setcontext(&sc);
+}
+
+int main(void)
+{
+	struct sigaction act;
+	stack_t stk;
+	int err;
+
+	sigemptyset(&act.sa_mask);
+	act.sa_flags = SA_ONSTACK | SA_SIGINFO;
+	act.sa_sigaction = my_usr1;
+	sigaction(SIGUSR1, &act, NULL);
+	act.sa_sigaction = my_usr2;
+	sigaction(SIGUSR2, &act, NULL);
+	sstack = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
+		      MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+	if (sstack == MAP_FAILED) {
+		perror("mmap()");
+		return EXIT_FAILURE;
+	}
+	stk.ss_sp = sstack;
+	stk.ss_size = SIGSTKSZ;
+	stk.ss_flags = SS_ONSTACK | SS_AUTODISARM;
+	err = sigaltstack(&stk, NULL);
+	if (err) {
+		perror("[FAIL]\tsigaltstack(SS_ONSTACK | SS_AUTODISARM)");
+		stk.ss_flags = SS_ONSTACK;
+	}
+	err = sigaltstack(&stk, NULL);
+	if (err) {
+		perror("[FAIL]\tsigaltstack(SS_ONSTACK)");
+		return EXIT_FAILURE;
+	}
+
+	ustack = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
+		      MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+	if (ustack == MAP_FAILED) {
+		perror("mmap()");
+		return EXIT_FAILURE;
+	}
+	getcontext(&uc);
+	uc.uc_link = NULL;
+	uc.uc_stack.ss_sp = ustack;
+	uc.uc_stack.ss_size = SIGSTKSZ;
+	makecontext(&uc, switch_fn, 0);
+	raise(SIGUSR1);
+
+	err = sigaltstack(NULL, &stk);
+	if (err) {
+		perror("[FAIL]\tsigaltstack()");
+		exit(EXIT_FAILURE);
+	}
+	if (stk.ss_flags != 0) {
+		printf("[FAIL]\tss_flags=%i, should be 0\n",
+				stk.ss_flags);
+		exit(EXIT_FAILURE);
+	}
+	printf("[OK]\tsigaltstack is enabled after signal\n");
+
+	printf("[OK]\tTest passed\n");
+	return 0;
+}
-- 
2.7.2

[tip:core/signals] signals/sigaltstack, x86/signals: Unify the x86 sigaltstack check with other architectures

[tip-bot for Stas Sergeev]

Commit-ID:  0b4521e8cf1f582da3045ea460427ac2f741578f
Gitweb:     http://git.kernel.org/tip/0b4521e8cf1f582da3045ea460427ac2f741578f
Author:     Stas Sergeev <stsp@list.ru>
AuthorDate: Thu, 14 Apr 2016 23:20:02 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 3 May 2016 08:37:58 +0200

signals/sigaltstack, x86/signals: Unify the x86 sigaltstack check with other architectures

Currently x86's get_sigframe() checks for "current->sas_ss_size"
to determine whether there is a need to switch to sigaltstack.
The common practice used by all other arches is to check for
sas_ss_flags(sp) == 0

This patch makes the code consistent with other architectures.

The slight complexity of the patch is added by the optimization on
!sigstack check that was requested by Andy Lutomirski: sas_ss_flags(sp)==0
already implies that we are not on a sigstack, so the code is shuffled
to avoid the duplicate checking.

This patch should have no user-visible impact.

Signed-off-by: Stas Sergeev <stsp@list.ru>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-api@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Link: http://lkml.kernel.org/r/1460665206-13646-2-git-send-email-stsp@list.ru
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/signal.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 548ddf7..3e84ef1 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -248,18 +248,17 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
 	if (config_enabled(CONFIG_X86_64))
 		sp -= 128;
 
-	if (!onsigstack) {
-		/* This is the X/Open sanctioned signal stack switching.  */
-		if (ka->sa.sa_flags & SA_ONSTACK) {
-			if (current->sas_ss_size)
-				sp = current->sas_ss_sp + current->sas_ss_size;
-		} else if (config_enabled(CONFIG_X86_32) &&
-			   (regs->ss & 0xffff) != __USER_DS &&
-			   !(ka->sa.sa_flags & SA_RESTORER) &&
-			   ka->sa.sa_restorer) {
-				/* This is the legacy signal stack switching. */
-				sp = (unsigned long) ka->sa.sa_restorer;
-		}
+	/* This is the X/Open sanctioned signal stack switching.  */
+	if (ka->sa.sa_flags & SA_ONSTACK) {
+		if (sas_ss_flags(sp) == 0)
+			sp = current->sas_ss_sp + current->sas_ss_size;
+	} else if (config_enabled(CONFIG_X86_32) &&
+		   !onsigstack &&
+		   (regs->ss & 0xffff) != __USER_DS &&
+		   !(ka->sa.sa_flags & SA_RESTORER) &&
+		   ka->sa.sa_restorer) {
+		/* This is the legacy signal stack switching. */
+		sp = (unsigned long) ka->sa.sa_restorer;
 	}
 
 	if (fpu->fpstate_active) {

[tip:core/signals] signals/sigaltstack: Prepare to add new SS_xxx flags

[tip-bot for Stas Sergeev]

Commit-ID:  407bc16ad1769f5cb8ad9555611cb198187ef4cd
Gitweb:     http://git.kernel.org/tip/407bc16ad1769f5cb8ad9555611cb198187ef4cd
Author:     Stas Sergeev <stsp@list.ru>
AuthorDate: Thu, 14 Apr 2016 23:20:03 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 3 May 2016 08:37:59 +0200

signals/sigaltstack: Prepare to add new SS_xxx flags

This patch adds SS_FLAG_BITS - the mask that splits sigaltstack
mode values and bit-flags. Since there is no bit-flags yet, the
mask is defined to 0. The flags are added by subsequent patches.
With every new flag, the mask should have the appropriate bit cleared.

This makes sure if some flag is tried on a kernel that doesn't
support it, the -EINVAL error will be returned, because such a
flag will be treated as an invalid mode rather than the bit-flag.

That way the existence of the particular features can be probed
at run-time.

This change was suggested by Andy Lutomirski:

  https://lkml.org/lkml/2016/3/6/158

Signed-off-by: Stas Sergeev <stsp@list.ru>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Amanieu d'Antras <amanieu@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vladimir Davydov <vdavydov@parallels.com>
Cc: linux-api@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Link: http://lkml.kernel.org/r/1460665206-13646-3-git-send-email-stsp@list.ru
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/uapi/linux/signal.h |  3 +++
 kernel/signal.c             | 16 ++++++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index e1bd50c2..7c73165 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -7,4 +7,7 @@
 #define SS_ONSTACK	1
 #define SS_DISABLE	2
 
+/* mask for all SS_xxx flags */
+#define SS_FLAG_BITS	0
+
 #endif /* _UAPI_LINUX_SIGNAL_H */
diff --git a/kernel/signal.c b/kernel/signal.c
index aa9bf00..b1c6eb4 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3104,7 +3104,8 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 	if (uss) {
 		void __user *ss_sp;
 		size_t ss_size;
-		int ss_flags;
+		unsigned ss_flags;
+		int ss_mode;
 
 		error = -EFAULT;
 		if (!access_ok(VERIFY_READ, uss, sizeof(*uss)))
@@ -3119,18 +3120,13 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 		if (on_sig_stack(sp))
 			goto out;
 
+		ss_mode = ss_flags & ~SS_FLAG_BITS;
 		error = -EINVAL;
-		/*
-		 * Note - this code used to test ss_flags incorrectly:
-		 *  	  old code may have been written using ss_flags==0
-		 *	  to mean ss_flags==SS_ONSTACK (as this was the only
-		 *	  way that worked) - this fix preserves that older
-		 *	  mechanism.
-		 */
-		if (ss_flags != SS_DISABLE && ss_flags != SS_ONSTACK && ss_flags != 0)
+		if (ss_mode != SS_DISABLE && ss_mode != SS_ONSTACK &&
+				ss_mode != 0)
 			goto out;
 
-		if (ss_flags == SS_DISABLE) {
+		if (ss_mode == SS_DISABLE) {
 			ss_size = 0;
 			ss_sp = NULL;
 		} else {

[tip:core/signals] signals/sigaltstack: Implement SS_AUTODISARM flag

[tip-bot for Stas Sergeev]

Commit-ID:  2a74213838104a41588d86fd5e8d344972891ace
Gitweb:     http://git.kernel.org/tip/2a74213838104a41588d86fd5e8d344972891ace
Author:     Stas Sergeev <stsp@list.ru>
AuthorDate: Thu, 14 Apr 2016 23:20:04 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 3 May 2016 08:37:59 +0200

signals/sigaltstack: Implement SS_AUTODISARM flag

This patch implements the SS_AUTODISARM flag that can be OR-ed with
SS_ONSTACK when forming ss_flags.

When this flag is set, sigaltstack will be disabled when entering
the signal handler; more precisely, after saving sas to uc_stack.
When leaving the signal handler, the sigaltstack is restored by
uc_stack.

When this flag is used, it is safe to switch from sighandler with
swapcontext(). Without this flag, the subsequent signal will corrupt
the state of the switched-away sighandler.

To detect the support of this functionality, one can do:

  err = sigaltstack(SS_DISABLE | SS_AUTODISARM);
  if (err && errno == EINVAL)
	unsupported();

Signed-off-by: Stas Sergeev <stsp@list.ru>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Amanieu d'Antras <amanieu@gmail.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Jason Low <jason.low2@hp.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Paul Moore <pmoore@redhat.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vladimir Davydov <vdavydov@parallels.com>
Cc: linux-api@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Link: http://lkml.kernel.org/r/1460665206-13646-4-git-send-email-stsp@list.ru
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/sched.h       |  8 ++++++++
 include/linux/signal.h      |  4 +++-
 include/uapi/linux/signal.h |  4 +++-
 kernel/fork.c               |  2 +-
 kernel/signal.c             | 10 ++++++++--
 5 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 52c4847..2950c5c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1596,6 +1596,7 @@ struct task_struct {
 
 	unsigned long sas_ss_sp;
 	size_t sas_ss_size;
+	unsigned sas_ss_flags;
 
 	struct callback_head *task_works;
 
@@ -2592,6 +2593,13 @@ static inline int sas_ss_flags(unsigned long sp)
 	return on_sig_stack(sp) ? SS_ONSTACK : 0;
 }
 
+static inline void sas_ss_reset(struct task_struct *p)
+{
+	p->sas_ss_sp = 0;
+	p->sas_ss_size = 0;
+	p->sas_ss_flags = SS_DISABLE;
+}
+
 static inline unsigned long sigsp(unsigned long sp, struct ksignal *ksig)
 {
 	if (unlikely((ksig->ka.sa.sa_flags & SA_ONSTACK)) && ! sas_ss_flags(sp))
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 92557bb..3fbe814 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -432,8 +432,10 @@ int __save_altstack(stack_t __user *, unsigned long);
 	stack_t __user *__uss = uss; \
 	struct task_struct *t = current; \
 	put_user_ex((void __user *)t->sas_ss_sp, &__uss->ss_sp); \
-	put_user_ex(sas_ss_flags(sp), &__uss->ss_flags); \
+	put_user_ex(t->sas_ss_flags, &__uss->ss_flags); \
 	put_user_ex(t->sas_ss_size, &__uss->ss_size); \
+	if (t->sas_ss_flags & SS_AUTODISARM) \
+		sas_ss_reset(t); \
 } while (0);
 
 #ifdef CONFIG_PROC_FS
diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index 7c73165..7388260 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -7,7 +7,9 @@
 #define SS_ONSTACK	1
 #define SS_DISABLE	2
 
+/* bit-flags */
+#define SS_AUTODISARM	(1 << 4)	/* disable sas during sighandling */
 /* mask for all SS_xxx flags */
-#define SS_FLAG_BITS	0
+#define SS_FLAG_BITS	SS_AUTODISARM
 
 #endif /* _UAPI_LINUX_SIGNAL_H */
diff --git a/kernel/fork.c b/kernel/fork.c
index d277e83..3e84515 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1494,7 +1494,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	 * sigaltstack should be cleared when sharing the same VM
 	 */
 	if ((clone_flags & (CLONE_VM|CLONE_VFORK)) == CLONE_VM)
-		p->sas_ss_sp = p->sas_ss_size = 0;
+		sas_ss_reset(p);
 
 	/*
 	 * Syscall tracing and stepping should be turned off in the
diff --git a/kernel/signal.c b/kernel/signal.c
index b1c6eb4..bf97ea5 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3137,6 +3137,7 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 
 		current->sas_ss_sp = (unsigned long) ss_sp;
 		current->sas_ss_size = ss_size;
+		current->sas_ss_flags = ss_flags;
 	}
 
 	error = 0;
@@ -3167,9 +3168,14 @@ int restore_altstack(const stack_t __user *uss)
 int __save_altstack(stack_t __user *uss, unsigned long sp)
 {
 	struct task_struct *t = current;
-	return  __put_user((void __user *)t->sas_ss_sp, &uss->ss_sp) |
-		__put_user(sas_ss_flags(sp), &uss->ss_flags) |
+	int err = __put_user((void __user *)t->sas_ss_sp, &uss->ss_sp) |
+		__put_user(t->sas_ss_flags, &uss->ss_flags) |
 		__put_user(t->sas_ss_size, &uss->ss_size);
+	if (err)
+		return err;
+	if (t->sas_ss_flags & SS_AUTODISARM)
+		sas_ss_reset(t);
+	return 0;
 }
 
 #ifdef CONFIG_COMPAT

[tip:core/signals] selftests/sigaltstack: Add new testcase for sigaltstack(SS_ONSTACK|SS_AUTODISARM)

[tip-bot for Stas Sergeev]

Commit-ID:  19fd2868e3671b446b13d135a44363182bbd319a
Gitweb:     http://git.kernel.org/tip/19fd2868e3671b446b13d135a44363182bbd319a
Author:     Stas Sergeev <stsp@list.ru>
AuthorDate: Thu, 14 Apr 2016 23:20:05 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 3 May 2016 08:37:59 +0200

selftests/sigaltstack: Add new testcase for sigaltstack(SS_ONSTACK|SS_AUTODISARM)

This patch adds the test case for SS_AUTODISARM flag.
The test-case tries to set SS_AUTODISARM flag and checks if
the nested signal corrupts the stack after swapcontext().

Signed-off-by: Stas Sergeev <stsp@list.ru>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-api@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Link: http://lkml.kernel.org/r/1460665206-13646-5-git-send-email-stsp@list.ru
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/testing/selftests/Makefile             |   1 +
 tools/testing/selftests/sigaltstack/Makefile |   8 ++
 tools/testing/selftests/sigaltstack/sas.c    | 156 +++++++++++++++++++++++++++
 3 files changed, 165 insertions(+)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index b04afc3..ff9e5f2 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -19,6 +19,7 @@ TARGETS += powerpc
 TARGETS += pstore
 TARGETS += ptrace
 TARGETS += seccomp
+TARGETS += sigaltstack
 TARGETS += size
 TARGETS += static_keys
 TARGETS += sysctl
diff --git a/tools/testing/selftests/sigaltstack/Makefile b/tools/testing/selftests/sigaltstack/Makefile
new file mode 100644
index 0000000..56af56e
--- /dev/null
+++ b/tools/testing/selftests/sigaltstack/Makefile
@@ -0,0 +1,8 @@
+CFLAGS = -Wall
+BINARIES = sas
+all: $(BINARIES)
+
+include ../lib.mk
+
+clean:
+	rm -rf $(BINARIES)
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
new file mode 100644
index 0000000..57da8bf
--- /dev/null
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -0,0 +1,156 @@
+/*
+ * Stas Sergeev <stsp@users.sourceforge.net>
+ *
+ * test sigaltstack(SS_ONSTACK | SS_AUTODISARM)
+ * If that succeeds, then swapcontext() can be used inside sighandler safely.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+#include <ucontext.h>
+#include <alloca.h>
+#include <string.h>
+#include <assert.h>
+
+#ifndef SS_AUTODISARM
+#define SS_AUTODISARM  (1 << 4)
+#endif
+
+static void *sstack, *ustack;
+static ucontext_t uc, sc;
+static const char *msg = "[OK]\tStack preserved";
+static const char *msg2 = "[FAIL]\tStack corrupted";
+struct stk_data {
+	char msg[128];
+	int flag;
+};
+
+void my_usr1(int sig, siginfo_t *si, void *u)
+{
+	char *aa;
+	int err;
+	stack_t stk;
+	struct stk_data *p;
+
+	register unsigned long sp asm("sp");
+
+	if (sp < (unsigned long)sstack ||
+			sp >= (unsigned long)sstack + SIGSTKSZ) {
+		printf("[FAIL]\tSP is not on sigaltstack\n");
+		exit(EXIT_FAILURE);
+	}
+	/* put some data on stack. other sighandler will try to overwrite it */
+	aa = alloca(1024);
+	assert(aa);
+	p = (struct stk_data *)(aa + 512);
+	strcpy(p->msg, msg);
+	p->flag = 1;
+	printf("[RUN]\tsignal USR1\n");
+	err = sigaltstack(NULL, &stk);
+	if (err) {
+		perror("[FAIL]\tsigaltstack()");
+		exit(EXIT_FAILURE);
+	}
+	if (stk.ss_flags != SS_DISABLE)
+		printf("[FAIL]\tss_flags=%i, should be SS_DISABLE\n",
+				stk.ss_flags);
+	else
+		printf("[OK]\tsigaltstack is disabled in sighandler\n");
+	swapcontext(&sc, &uc);
+	printf("%s\n", p->msg);
+	if (!p->flag) {
+		printf("[RUN]\tAborting\n");
+		exit(EXIT_FAILURE);
+	}
+}
+
+void my_usr2(int sig, siginfo_t *si, void *u)
+{
+	char *aa;
+	struct stk_data *p;
+
+	printf("[RUN]\tsignal USR2\n");
+	aa = alloca(1024);
+	/* dont run valgrind on this */
+	/* try to find the data stored by previous sighandler */
+	p = memmem(aa, 1024, msg, strlen(msg));
+	if (p) {
+		printf("[FAIL]\tsigaltstack re-used\n");
+		/* corrupt the data */
+		strcpy(p->msg, msg2);
+		/* tell other sighandler that his data is corrupted */
+		p->flag = 0;
+	}
+}
+
+static void switch_fn(void)
+{
+	printf("[RUN]\tswitched to user ctx\n");
+	raise(SIGUSR2);
+	setcontext(&sc);
+}
+
+int main(void)
+{
+	struct sigaction act;
+	stack_t stk;
+	int err;
+
+	sigemptyset(&act.sa_mask);
+	act.sa_flags = SA_ONSTACK | SA_SIGINFO;
+	act.sa_sigaction = my_usr1;
+	sigaction(SIGUSR1, &act, NULL);
+	act.sa_sigaction = my_usr2;
+	sigaction(SIGUSR2, &act, NULL);
+	sstack = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
+		      MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+	if (sstack == MAP_FAILED) {
+		perror("mmap()");
+		return EXIT_FAILURE;
+	}
+	stk.ss_sp = sstack;
+	stk.ss_size = SIGSTKSZ;
+	stk.ss_flags = SS_ONSTACK | SS_AUTODISARM;
+	err = sigaltstack(&stk, NULL);
+	if (err) {
+		perror("[FAIL]\tsigaltstack(SS_ONSTACK | SS_AUTODISARM)");
+		stk.ss_flags = SS_ONSTACK;
+	}
+	err = sigaltstack(&stk, NULL);
+	if (err) {
+		perror("[FAIL]\tsigaltstack(SS_ONSTACK)");
+		return EXIT_FAILURE;
+	}
+
+	ustack = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
+		      MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+	if (ustack == MAP_FAILED) {
+		perror("mmap()");
+		return EXIT_FAILURE;
+	}
+	getcontext(&uc);
+	uc.uc_link = NULL;
+	uc.uc_stack.ss_sp = ustack;
+	uc.uc_stack.ss_size = SIGSTKSZ;
+	makecontext(&uc, switch_fn, 0);
+	raise(SIGUSR1);
+
+	err = sigaltstack(NULL, &stk);
+	if (err) {
+		perror("[FAIL]\tsigaltstack()");
+		exit(EXIT_FAILURE);
+	}
+	if (stk.ss_flags != 0) {
+		printf("[FAIL]\tss_flags=%i, should be 0\n",
+				stk.ss_flags);
+		exit(EXIT_FAILURE);
+	}
+	printf("[OK]\tsigaltstack is enabled after signal\n");
+
+	printf("[OK]\tTest passed\n");
+	return 0;
+}

Re: [tip:core/signals] signals/sigaltstack: Prepare to add new SS_xxx flags

[Borislav Petkov]

On Tue, May 03, 2016 at 12:50:06AM -0700, tip-bot for Stas Sergeev wrote:
> Commit-ID:  407bc16ad1769f5cb8ad9555611cb198187ef4cd
> Gitweb:     http://git.kernel.org/tip/407bc16ad1769f5cb8ad9555611cb198187ef4cd
> Author:     Stas Sergeev <stsp@list.ru>
> AuthorDate: Thu, 14 Apr 2016 23:20:03 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Tue, 3 May 2016 08:37:59 +0200
> 
> signals/sigaltstack: Prepare to add new SS_xxx flags
> 
> This patch adds SS_FLAG_BITS - the mask that splits sigaltstack
> mode values and bit-flags. Since there is no bit-flags yet, the
> mask is defined to 0. The flags are added by subsequent patches.
> With every new flag, the mask should have the appropriate bit cleared.
> 
> This makes sure if some flag is tried on a kernel that doesn't
> support it, the -EINVAL error will be returned, because such a
> flag will be treated as an invalid mode rather than the bit-flag.
> 
> That way the existence of the particular features can be probed
> at run-time.
> 
> This change was suggested by Andy Lutomirski:
> 
>   https://lkml.org/lkml/2016/3/6/158

Please use the message ID in the future

Message-ID: <CALCETrW=M1=n6R6dwOj-ks_=+14vf4rz0S3SFty-_GuwFFZU5A@mail.gmail.com>

to refer to public mails because random web services caching lkml tend
to become unreliable or even disappear at some point.

I.e., see Link: format below for an example.

> Signed-off-by: Stas Sergeev <stsp@list.ru>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Amanieu d'Antras <amanieu@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Brian Gerst <brgerst@gmail.com>
> Cc: Denys Vlasenko <dvlasenk@redhat.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vladimir Davydov <vdavydov@parallels.com>
> Cc: linux-api@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Link: http://lkml.kernel.org/r/1460665206-13646-3-git-send-email-stsp@list.ru
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  include/uapi/linux/signal.h |  3 +++
>  kernel/signal.c             | 16 ++++++----------
>  2 files changed, 9 insertions(+), 10 deletions(-)

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [tip:core/signals] signals/sigaltstack, x86/signals: Unify the x86 sigaltstack check with other architectures

[Andy Lutomirski]

On Tue, May 3, 2016 at 12:49 AM, tip-bot for Stas Sergeev
<tipbot@zytor.com> wrote:
> Commit-ID:  0b4521e8cf1f582da3045ea460427ac2f741578f
> Gitweb:     http://git.kernel.org/tip/0b4521e8cf1f582da3045ea460427ac2f741578f
> Author:     Stas Sergeev <stsp@list.ru>
> AuthorDate: Thu, 14 Apr 2016 23:20:02 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Tue, 3 May 2016 08:37:58 +0200
>
> signals/sigaltstack, x86/signals: Unify the x86 sigaltstack check with other architectures
>
> Currently x86's get_sigframe() checks for "current->sas_ss_size"
> to determine whether there is a need to switch to sigaltstack.
> The common practice used by all other arches is to check for
> sas_ss_flags(sp) == 0
>
> This patch makes the code consistent with other architectures.
>
> The slight complexity of the patch is added by the optimization on
> !sigstack check that was requested by Andy Lutomirski: sas_ss_flags(sp)==0
> already implies that we are not on a sigstack, so the code is shuffled
> to avoid the duplicate checking.
>
> This patch should have no user-visible impact.

I'm late to the party, but LGTM.

--Andy

Re: [tip:core/signals] signals/sigaltstack: Prepare to add new SS_xxx flags

[Andy Lutomirski]

On Tue, May 3, 2016 at 12:50 AM, tip-bot for Stas Sergeev
<tipbot@zytor.com> wrote:
> Commit-ID:  407bc16ad1769f5cb8ad9555611cb198187ef4cd
> Gitweb:     http://git.kernel.org/tip/407bc16ad1769f5cb8ad9555611cb198187ef4cd
> Author:     Stas Sergeev <stsp@list.ru>
> AuthorDate: Thu, 14 Apr 2016 23:20:03 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Tue, 3 May 2016 08:37:59 +0200
>
> signals/sigaltstack: Prepare to add new SS_xxx flags
>
> This patch adds SS_FLAG_BITS - the mask that splits sigaltstack
> mode values and bit-flags. Since there is no bit-flags yet, the
> mask is defined to 0. The flags are added by subsequent patches.
> With every new flag, the mask should have the appropriate bit cleared.
>
> This makes sure if some flag is tried on a kernel that doesn't
> support it, the -EINVAL error will be returned, because such a
> flag will be treated as an invalid mode rather than the bit-flag.
>
> That way the existence of the particular features can be probed
> at run-time.
>
> This change was suggested by Andy Lutomirski:
>
>   https://lkml.org/lkml/2016/3/6/158

LGTM.

Re: [tip:core/signals] signals/sigaltstack: Implement SS_AUTODISARM flag

[Andy Lutomirski]

On Tue, May 3, 2016 at 12:50 AM, tip-bot for Stas Sergeev
<tipbot@zytor.com> wrote:
> Commit-ID:  2a74213838104a41588d86fd5e8d344972891ace
> Gitweb:     http://git.kernel.org/tip/2a74213838104a41588d86fd5e8d344972891ace
> Author:     Stas Sergeev <stsp@list.ru>
> AuthorDate: Thu, 14 Apr 2016 23:20:04 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Tue, 3 May 2016 08:37:59 +0200
>
> signals/sigaltstack: Implement SS_AUTODISARM flag
>
> This patch implements the SS_AUTODISARM flag that can be OR-ed with
> SS_ONSTACK when forming ss_flags.
>
> When this flag is set, sigaltstack will be disabled when entering
> the signal handler; more precisely, after saving sas to uc_stack.
> When leaving the signal handler, the sigaltstack is restored by
> uc_stack.
>
> When this flag is used, it is safe to switch from sighandler with
> swapcontext(). Without this flag, the subsequent signal will corrupt
> the state of the switched-away sighandler.
>
> To detect the support of this functionality, one can do:
>
>   err = sigaltstack(SS_DISABLE | SS_AUTODISARM);
>   if (err && errno == EINVAL)
>         unsupported();

One of my review comments from last time got lost, I think.  I'll send
a followup patch.

> +/* bit-flags */
> +#define SS_AUTODISARM  (1 << 4)        /* disable sas during sighandling */

Before this gets enshrined as ABI, could we perhaps change this to (1
<< 31)?  I don't see why we should pick a bit in the middle of the
field as our first flag bit.

--Andy

Re: [tip:core/signals] selftests/sigaltstack: Add new testcase for sigaltstack(SS_ONSTACK|SS_AUTODISARM)

[Andy Lutomirski]

On Tue, May 3, 2016 at 12:50 AM, tip-bot for Stas Sergeev
<tipbot@zytor.com> wrote:
> Commit-ID:  19fd2868e3671b446b13d135a44363182bbd319a
> Gitweb:     http://git.kernel.org/tip/19fd2868e3671b446b13d135a44363182bbd319a
> Author:     Stas Sergeev <stsp@list.ru>
> AuthorDate: Thu, 14 Apr 2016 23:20:05 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Tue, 3 May 2016 08:37:59 +0200
>
> selftests/sigaltstack: Add new testcase for sigaltstack(SS_ONSTACK|SS_AUTODISARM)
>
> This patch adds the test case for SS_AUTODISARM flag.
> The test-case tries to set SS_AUTODISARM flag and checks if
> the nested signal corrupts the stack after swapcontext().


> +       stk.ss_sp = sstack;
> +       stk.ss_size = SIGSTKSZ;
> +       stk.ss_flags = SS_ONSTACK | SS_AUTODISARM;
> +       err = sigaltstack(&stk, NULL);
> +       if (err) {
> +               perror("[FAIL]\tsigaltstack(SS_ONSTACK | SS_AUTODISARM)");
> +               stk.ss_flags = SS_ONSTACK;

This code path here is wrong.  I'll send a fix.

--Andy

Re: [tip:core/signals] signals/sigaltstack: Prepare to add new SS_xxx flags

[Andy Lutomirski]

On Tue, May 3, 2016 at 9:13 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Tue, May 3, 2016 at 12:50 AM, tip-bot for Stas Sergeev
> <tipbot@zytor.com> wrote:
>> Commit-ID:  407bc16ad1769f5cb8ad9555611cb198187ef4cd
>> Gitweb:     http://git.kernel.org/tip/407bc16ad1769f5cb8ad9555611cb198187ef4cd
>> Author:     Stas Sergeev <stsp@list.ru>
>> AuthorDate: Thu, 14 Apr 2016 23:20:03 +0300
>> Committer:  Ingo Molnar <mingo@kernel.org>
>> CommitDate: Tue, 3 May 2016 08:37:59 +0200
>>
>> signals/sigaltstack: Prepare to add new SS_xxx flags
>>
>> This patch adds SS_FLAG_BITS - the mask that splits sigaltstack
>> mode values and bit-flags. Since there is no bit-flags yet, the
>> mask is defined to 0. The flags are added by subsequent patches.
>> With every new flag, the mask should have the appropriate bit cleared.
>>
>> This makes sure if some flag is tried on a kernel that doesn't
>> support it, the -EINVAL error will be returned, because such a
>> flag will be treated as an invalid mode rather than the bit-flag.
>>
>> That way the existence of the particular features can be probed
>> at run-time.
>>
>> This change was suggested by Andy Lutomirski:
>>
>>   https://lkml.org/lkml/2016/3/6/158
>
> LGTM.

Correction: does not LGTM.  In a sensible design, sigaltstack would
report flags back to the caller.  I will send a fix.

--Andy

[PATCH 2/4] sigaltstack: preparations for adding new SS_xxx flags

[Stas Sergeev]

This patch adds SS_FLAG_BITS - the mask that splits sigaltstack
mode values and bit-flags. Since there is no bit-flags yet, the
mask is defined to 0. The flags are added by subsequent patches.
With every new flag, the mask should have the appropriate bit cleared.

This makes sure if some flag is tried on a kernel that doesn't
support it, the EINVAL error will be returned, because such a
flag will be treated as an invalid mode rather than the bit-flag.
That way the existence of the particular features can be probed
at run-time.

This change was suggested by Andy Lutomirski:
https://lkml.org/lkml/2016/3/6/158

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>

CC: Andrew Morton <akpm@linux-foundation.org>
CC: Oleg Nesterov <oleg@redhat.com>
CC: "Peter Zijlstra (Intel)" <peterz@infradead.org>
CC: "Amanieu d'Antras" <amanieu@gmail.com>
CC: Michal Hocko <mhocko@suse.com>
CC: Richard Weinberger <richard@nod.at>
CC: Vladimir Davydov <vdavydov@parallels.com>
CC: Sasha Levin <sasha.levin@oracle.com>
CC: linux-kernel@vger.kernel.org
---
 include/uapi/linux/signal.h |  3 +++
 kernel/signal.c             | 16 ++++++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index e1bd50c2..7c73165 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -7,4 +7,7 @@
 #define SS_ONSTACK	1
 #define SS_DISABLE	2
 
+/* mask for all SS_xxx flags */
+#define SS_FLAG_BITS	0
+
 #endif /* _UAPI_LINUX_SIGNAL_H */
diff --git a/kernel/signal.c b/kernel/signal.c
index 0508544..9a24bc3 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3100,7 +3100,8 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 	if (uss) {
 		void __user *ss_sp;
 		size_t ss_size;
-		int ss_flags;
+		unsigned ss_flags;
+		int ss_mode;
 
 		error = -EFAULT;
 		if (!access_ok(VERIFY_READ, uss, sizeof(*uss)))
@@ -3115,18 +3116,13 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 		if (on_sig_stack(sp))
 			goto out;
 
+		ss_mode = ss_flags & ~SS_FLAG_BITS;
 		error = -EINVAL;
-		/*
-		 * Note - this code used to test ss_flags incorrectly:
-		 *  	  old code may have been written using ss_flags==0
-		 *	  to mean ss_flags==SS_ONSTACK (as this was the only
-		 *	  way that worked) - this fix preserves that older
-		 *	  mechanism.
-		 */
-		if (ss_flags != SS_DISABLE && ss_flags != SS_ONSTACK && ss_flags != 0)
+		if (ss_mode != SS_DISABLE && ss_mode != SS_ONSTACK &&
+				ss_mode != 0)
 			goto out;
 
-		if (ss_flags == SS_DISABLE) {
+		if (ss_mode == SS_DISABLE) {
 			ss_size = 0;
 			ss_sp = NULL;
 		} else {
-- 
2.7.2

[PATCH 2/4] sigaltstack: preparations for adding new SS_xxx flags

[Stas Sergeev]

This patch adds SS_FLAG_BITS - the mask that splits sigaltstack
mode values and bit-flags. Since there is no bit-flags yet, the
mask is defined to 0. The flags are added by subsequent patches.
With every new flag, the mask should have the appropriate bit cleared.

This makes sure if some flag is tried on a kernel that doesn't
support it, the EINVAL error will be returned, because such a
flag will be treated as an invalid mode rather than the bit-flag.
That way the existence of the particular features can be probed
at run-time.

This change was suggested by Andy Lutomirski:
https://lkml.org/lkml/2016/3/6/158

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>

CC: Andrew Morton <akpm@linux-foundation.org>
CC: Oleg Nesterov <oleg@redhat.com>
CC: "Peter Zijlstra (Intel)" <peterz@infradead.org>
CC: "Amanieu d'Antras" <amanieu@gmail.com>
CC: Michal Hocko <mhocko@suse.com>
CC: Richard Weinberger <richard@nod.at>
CC: Vladimir Davydov <vdavydov@parallels.com>
CC: Sasha Levin <sasha.levin@oracle.com>
CC: linux-kernel@vger.kernel.org
---
 include/uapi/linux/signal.h |  3 +++
 kernel/signal.c             | 16 ++++++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index e1bd50c2..7c73165 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -7,4 +7,7 @@
 #define SS_ONSTACK	1
 #define SS_DISABLE	2
 
+/* mask for all SS_xxx flags */
+#define SS_FLAG_BITS	0
+
 #endif /* _UAPI_LINUX_SIGNAL_H */
diff --git a/kernel/signal.c b/kernel/signal.c
index 0508544..9a24bc3 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3100,7 +3100,8 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 	if (uss) {
 		void __user *ss_sp;
 		size_t ss_size;
-		int ss_flags;
+		unsigned ss_flags;
+		int ss_mode;
 
 		error = -EFAULT;
 		if (!access_ok(VERIFY_READ, uss, sizeof(*uss)))
@@ -3115,18 +3116,13 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 		if (on_sig_stack(sp))
 			goto out;
 
+		ss_mode = ss_flags & ~SS_FLAG_BITS;
 		error = -EINVAL;
-		/*
-		 * Note - this code used to test ss_flags incorrectly:
-		 *  	  old code may have been written using ss_flags==0
-		 *	  to mean ss_flags==SS_ONSTACK (as this was the only
-		 *	  way that worked) - this fix preserves that older
-		 *	  mechanism.
-		 */
-		if (ss_flags != SS_DISABLE && ss_flags != SS_ONSTACK && ss_flags != 0)
+		if (ss_mode != SS_DISABLE && ss_mode != SS_ONSTACK &&
+				ss_mode != 0)
 			goto out;
 
-		if (ss_flags == SS_DISABLE) {
+		if (ss_mode == SS_DISABLE) {
 			ss_size = 0;
 			ss_sp = NULL;
 		} else {
-- 
2.7.2

[PATCH 2/4] sigaltstack: preparations for adding new SS_xxx flags

[Stas Sergeev]

This patch adds SS_FLAG_BITS - the mask that splits sigaltstack
mode values and bit-flags. Since there is no bit-flags yet, the
mask is defined to 0. The flags are added by subsequent patches.
With every new flag, the mask should have the appropriate bit cleared.

This makes sure if some flag is tried on a kernel that doesn't
support it, the EINVAL error will be returned, because such a
flag will be treated as an invalid mode rather than the bit-flag.
That way the existence of the particular features can be probed
at run-time.

This change was suggested by Andy Lutomirski:
https://lkml.org/lkml/2016/3/6/158

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>

CC: Andrew Morton <akpm@linux-foundation.org>
CC: Oleg Nesterov <oleg@redhat.com>
CC: "Peter Zijlstra (Intel)" <peterz@infradead.org>
CC: "Amanieu d'Antras" <amanieu@gmail.com>
CC: Michal Hocko <mhocko@suse.com>
CC: Richard Weinberger <richard@nod.at>
CC: Vladimir Davydov <vdavydov@parallels.com>
CC: Sasha Levin <sasha.levin@oracle.com>
CC: linux-kernel@vger.kernel.org
---
 include/uapi/linux/signal.h |  3 +++
 kernel/signal.c             | 16 ++++++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index e1bd50c2..7c73165 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -7,4 +7,7 @@
 #define SS_ONSTACK	1
 #define SS_DISABLE	2
 
+/* mask for all SS_xxx flags */
+#define SS_FLAG_BITS	0
+
 #endif /* _UAPI_LINUX_SIGNAL_H */
diff --git a/kernel/signal.c b/kernel/signal.c
index 0508544..9a24bc3 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3100,7 +3100,8 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 	if (uss) {
 		void __user *ss_sp;
 		size_t ss_size;
-		int ss_flags;
+		unsigned ss_flags;
+		int ss_mode;
 
 		error = -EFAULT;
 		if (!access_ok(VERIFY_READ, uss, sizeof(*uss)))
@@ -3115,18 +3116,13 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 		if (on_sig_stack(sp))
 			goto out;
 
+		ss_mode = ss_flags & ~SS_FLAG_BITS;
 		error = -EINVAL;
-		/*
-		 * Note - this code used to test ss_flags incorrectly:
-		 *  	  old code may have been written using ss_flags==0
-		 *	  to mean ss_flags==SS_ONSTACK (as this was the only
-		 *	  way that worked) - this fix preserves that older
-		 *	  mechanism.
-		 */
-		if (ss_flags != SS_DISABLE && ss_flags != SS_ONSTACK && ss_flags != 0)
+		if (ss_mode != SS_DISABLE && ss_mode != SS_ONSTACK &&
+				ss_mode != 0)
 			goto out;
 
-		if (ss_flags == SS_DISABLE) {
+		if (ss_mode == SS_DISABLE) {
 			ss_size = 0;
 			ss_sp = NULL;
 		} else {
-- 
2.7.2