[PATCH v4 0/2] make sigaltstack() compatible with swapcontext()

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.

[PATCH 1/2] sigaltstack: implement SS_AUTODISARM flag
	This patch implements SS_AUTODISARM flag
[PATCH 2/2] selftests: Add test for sigaltstack(SS_AUTODISARM)
	This patch adds the selftest code for new functionality

CC: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Andy Lutomirski <redacted>
CC: Oleg Nesterov <redacted>
CC: Shuah Khan <redacted>

Diffstat:
 include/linux/sched.h                        |    8 +
 include/linux/signal.h                       |    4 
 include/uapi/linux/signal.h                  |    3 
 kernel/fork.c                                |    2 
 kernel/signal.c                              |   23 ++--
 tools/testing/selftests/Makefile             |    1 
 tools/testing/selftests/sigaltstack/Makefile |    8 +
 tools/testing/selftests/sigaltstack/sas.c    |  151 +++++++++++++++++++++++++++
 8 files changed, 187 insertions(+), 13 deletions(-)

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.

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 <redacted>
CC: Jason Low <redacted>
CC: Andrea Arcangeli <redacted>
CC: Frederic Weisbecker <redacted>
CC: Konstantin Khlebnikov <redacted>
CC: Josh Triplett <josh@joshtriplett.org>
CC: "Eric W. Biederman" <redacted>
CC: Aleksa Sarai <redacted>
CC: "Amanieu d'Antras" <redacted>
CC: Paul Moore <redacted>
CC: Sasha Levin <redacted>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: Vladimir Davydov <redacted>
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: Andy Lutomirski <luto@amacapital.net>

Signed-off-by: Stas Sergeev <redacted>
---
 include/linux/sched.h       |  8 ++++++++
 include/linux/signal.h      |  4 +++-
 include/uapi/linux/signal.h |  3 +++
 kernel/fork.c               |  2 +-
 kernel/signal.c             | 23 ++++++++++++-----------
 5 files changed, 27 insertions(+), 13 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 e1bd50c2..4691bc5 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h

@@ -6,5 +6,8 @@
 
 #define SS_ONSTACK	1
 #define SS_DISABLE	2
+#define SS_VALMASK	0xf
+/* bit-flags */
+#define SS_AUTODISARM	(1 << 4)	/* disable sas during sighandling */
 
 #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 0508544..fab6bea 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c

@@ -3100,13 +3100,14 @@ 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;
+		unsigned ss_xflags;
 
 		error = -EFAULT;
 		if (!access_ok(VERIFY_READ, uss, sizeof(*uss)))
 			goto out;
 		error = __get_user(ss_sp, &uss->ss_sp) |
-			__get_user(ss_flags, &uss->ss_flags) |
+			__get_user(ss_xflags, &uss->ss_flags) |
 			__get_user(ss_size, &uss->ss_size);
 		if (error)
 			goto out;

@@ -3115,14 +3116,8 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 		if (on_sig_stack(sp))
 			goto out;
 
+		ss_flags = ss_xflags & SS_VALMASK;
 		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)
 			goto out;
 

@@ -3137,6 +3132,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_xflags;
 	}
 
 	error = 0;

@@ -3167,9 +3163,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

From: Stas Sergeev <redacted>

sigaltstack needs to be disabled before the signal handler can
safely use swapcontext().
This patch adds the SS_AUTODISARM flag.
This flag disables the sigaltstack when entering the signal handler.
When returning from signal handler, the sigaltstack is restored by
uc_stack.

CC: Shuah Khan <redacted>
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: Andy Lutomirski <luto@amacapital.net>

Signed-off-by: Stas Sergeev <redacted>
---
 tools/testing/selftests/Makefile             |   1 +
 tools/testing/selftests/sigaltstack/Makefile |   8 ++
 tools/testing/selftests/sigaltstack/sas.c    | 151 +++++++++++++++++++++++++++
 3 files changed, 160 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..bba149a
--- /dev/null
+++ b/tools/testing/selftests/sigaltstack/sas.c

@@ -0,0 +1,151 @@
+/*
+ * Stas Sergeev <stsp@users.sourceforge.net>
+ *
+ * test sigaltstack(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("sigaltstack()");
+		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

On Mon, Feb 29, 2016 at 1:29 PM, Stas Sergeev [off-list ref] wrote:

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.

This looks reasonable to me with one exception: how does a user
program detect the presence of this feature?  Anyone else have any
thoughts?

05.03.2016 01:22, Andy Lutomirski пишет:

On Mon, Feb 29, 2016 at 1:29 PM, Stas Sergeev [off-list ref] wrote:

quoted

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.

This looks reasonable to me with one exception: how does a user
program detect the presence of this feature?

Compile-time detection:
#ifdef SS_AUTODISARM
   # I have this feature
   ...
#endif

Run-time detection:
int err = sigaltstack(SS_ONSTACK | SS_AUTODISARM);
if (err == EINVAL) {
   i_dont_have_this_feature = 1;
   err = sigaltstack(SS_ONSTACK);
}

Note: if you want to keep such detection for the future
additions, the mask can be enlarged to, say, ((1 << 24) - 1),
and whenever someone adds a new flag, he can lower the
mask by one bit.
But I think this would be an overkill in that particular case.

05.03.2016 10:39, Stas Sergeev пишет:

05.03.2016 01:22, Andy Lutomirski пишет:

quoted

On Mon, Feb 29, 2016 at 1:29 PM, Stas Sergeev [off-list ref] wrote:

quoted

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.

This looks reasonable to me with one exception: how does a user
program detect the presence of this feature?

Compile-time detection:
#ifdef SS_AUTODISARM
  # I have this feature
  ...
#endif

Run-time detection:
int err = sigaltstack(SS_ONSTACK | SS_AUTODISARM);
if (err == EINVAL) {

I guess I mean here
if (err && errno == EINVAL)

On Mon, Feb 29, 2016 at 1:29 PM, Stas Sergeev [off-list ref] wrote:

SS_MODE_MASK, perhaps?

This is poorly named.  How about "ss_mode = ss_flags & SS_MODE_MASK";

--Andy

* Stas Sergeev [off-list ref] [2016-03-01 00:29:03 +0300]:

The following patches make it possible to use swapcontext()
in a sighandler that works on sigaltstack.

i don't think that's possible, the (obsolete) userspace
*context functions cannot operate on kernel provided
ucontext_t structs, so they are not usable in signal
handlers.

at least not on most target archs.

so i don't understand how the tests can work portably.

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.

[PATCH 1/2] sigaltstack: implement SS_AUTODISARM flag
	This patch implements SS_AUTODISARM flag
[PATCH 2/2] selftests: Add test for sigaltstack(SS_AUTODISARM)
	This patch adds the selftest code for new functionality

CC: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Andy Lutomirski <redacted>
CC: Oleg Nesterov <redacted>
CC: Shuah Khan <redacted>

Diffstat:
 include/linux/sched.h                        |    8 +
 include/linux/signal.h                       |    4 
 include/uapi/linux/signal.h                  |    3 
 kernel/fork.c                                |    2 
 kernel/signal.c                              |   23 ++--
 tools/testing/selftests/Makefile             |    1 
 tools/testing/selftests/sigaltstack/Makefile |    8 +
 tools/testing/selftests/sigaltstack/sas.c    |  151 +++++++++++++++++++++++++++
 8 files changed, 187 insertions(+), 13 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sun, Mar 6, 2016 at 12:07 PM, Andy Lutomirski [off-list ref] wrote:

On Mon, Feb 29, 2016 at 1:29 PM, Stas Sergeev [off-list ref] wrote:

quoted

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.

CC: Ingo Molnar <redacted>
CC: Peter Zijlstra <redacted>
CC: Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org>
CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: Oleg Nesterov <redacted>
CC: Tejun Heo <redacted>
CC: Heinrich Schuchardt <redacted>
CC: Jason Low <redacted>
CC: Andrea Arcangeli <redacted>
CC: Frederic Weisbecker <redacted>
CC: Konstantin Khlebnikov <redacted>
CC: Josh Triplett <redacted>
CC: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
CC: Aleksa Sarai <cyphar-gVpy/LI/lHzQT0dZR+AlfA@public.gmane.org>
CC: "Amanieu d'Antras" <redacted>
CC: Paul Moore <redacted>
CC: Sasha Levin <redacted>
CC: Palmer Dabbelt <redacted>
CC: Vladimir Davydov <redacted>
CC: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Andy Lutomirski <redacted>

Signed-off-by: Stas Sergeev <redacted>
---
 include/linux/sched.h       |  8 ++++++++
 include/linux/signal.h      |  4 +++-
 include/uapi/linux/signal.h |  3 +++
 kernel/fork.c               |  2 +-
 kernel/signal.c             | 23 ++++++++++++-----------
 5 files changed, 27 insertions(+), 13 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 e1bd50c2..4691bc5 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h

@@ -6,5 +6,8 @@

 #define SS_ONSTACK     1
 #define SS_DISABLE     2
+#define SS_VALMASK     0xf

SS_MODE_MASK, perhaps?

Actually, let's invert that.

#define SS_AUTODISARM (1U << 31)
#define SS_FLAG_BITS SS_AUTODISARM

ss_mode = ss_flags & ~SS_FLAG_BITS;

this way flag bits that are currently undefined will continue to trigger EINVAL.

--Andy

06.03.2016 23:10, Andy Lutomirski пишет:

On Sun, Mar 6, 2016 at 12:07 PM, Andy Lutomirski [off-list ref] wrote:

quoted

On Mon, Feb 29, 2016 at 1:29 PM, Stas Sergeev [off-list ref] wrote:

quoted

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.

CC: Ingo Molnar <redacted>
CC: Peter Zijlstra <redacted>
CC: Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org>
CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: Oleg Nesterov <redacted>
CC: Tejun Heo <redacted>
CC: Heinrich Schuchardt <redacted>
CC: Jason Low <redacted>
CC: Andrea Arcangeli <redacted>
CC: Frederic Weisbecker <redacted>
CC: Konstantin Khlebnikov <redacted>
CC: Josh Triplett <redacted>
CC: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
CC: Aleksa Sarai <cyphar-gVpy/LI/lHzQT0dZR+AlfA@public.gmane.org>
CC: "Amanieu d'Antras" <redacted>
CC: Paul Moore <redacted>
CC: Sasha Levin <redacted>
CC: Palmer Dabbelt <redacted>
CC: Vladimir Davydov <redacted>
CC: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Andy Lutomirski <redacted>

Signed-off-by: Stas Sergeev <redacted>
---
  include/linux/sched.h       |  8 ++++++++
  include/linux/signal.h      |  4 +++-
  include/uapi/linux/signal.h |  3 +++
  kernel/fork.c               |  2 +-
  kernel/signal.c             | 23 ++++++++++++-----------
  5 files changed, 27 insertions(+), 13 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 e1bd50c2..4691bc5 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h

@@ -6,5 +6,8 @@

  #define SS_ONSTACK     1
  #define SS_DISABLE     2
+#define SS_VALMASK     0xf

SS_MODE_MASK, perhaps?

Actually, let's invert that.

#define SS_AUTODISARM (1U << 31)
#define SS_FLAG_BITS SS_AUTODISARM

ss_mode = ss_flags & ~SS_FLAG_BITS;

this way flag bits that are currently undefined will continue to trigger EINVAL.

OK, will do.
Though I am still going to keep SS_AUTODISARM =(1 << 4) -
it doesn't matter from what side we add new flags, does it?

06.03.2016 23:02, Szabolcs Nagy пишет:

* Stas Sergeev [off-list ref] [2016-03-01 00:29:03 +0300]:

quoted

The following patches make it possible to use swapcontext()
in a sighandler that works on sigaltstack.

i don't think that's possible, the (obsolete) userspace
*context functions cannot operate on kernel provided
ucontext_t structs,

It doesn't have to.
It does the normal getcontext()/swapcontext() sequences,
as if it would be switching from any casual code place.
Kernel-provided context structs are completely irrelevant.
For more details please see the test-case I posted in that patch series.
It shows what can be done and how.

  so they are not usable in signal
handlers.

Please note the fact that swapcontext() also replaces the
signal mask as part of the context switching. This is a very
strong hint that it was intended to work with signal handlers.
And in fact it perfectly does.
What I want is only to make it friendly to sigaltstack.
I am not making it friendly to signal handlers in general,
because it already is.

at least not on most target archs.

so i don't understand how the tests can work portably.

If you apply my patch series, you can immediately test
it on any arch you want, thanks to the test-case my patch
adds to the kernel. Please let me know on what arch it fails.

* Szabolcs Nagy [off-list ref] [2016-03-06 21:02:08 +0100]:

* Stas Sergeev [off-list ref] [2016-03-01 00:29:03 +0300]:

quoted

The following patches make it possible to use swapcontext()
in a sighandler that works on sigaltstack.

i don't think that's possible, the (obsolete) userspace
*context functions cannot operate on kernel provided
ucontext_t structs, so they are not usable in signal
handlers.

at least not on most target archs.

so i don't understand how the tests can work portably.

i see the test does not use the kernel ucontext_t
and only synchronous context switch is done

sorry for the noise.

On Sun, Mar 6, 2016 at 12:17 PM, Stas Sergeev [off-list ref] wrote:

06.03.2016 23:10, Andy Lutomirski пишет:

quoted

On Sun, Mar 6, 2016 at 12:07 PM, Andy Lutomirski [off-list ref]
wrote:

quoted

On Mon, Feb 29, 2016 at 1:29 PM, Stas Sergeev [off-list ref] wrote:

quoted

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.

CC: Ingo Molnar <redacted>
CC: Peter Zijlstra <redacted>
CC: Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org>
CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: Oleg Nesterov <redacted>
CC: Tejun Heo <redacted>
CC: Heinrich Schuchardt <redacted>
CC: Jason Low <redacted>
CC: Andrea Arcangeli <redacted>
CC: Frederic Weisbecker <redacted>
CC: Konstantin Khlebnikov <redacted>
CC: Josh Triplett <redacted>
CC: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
CC: Aleksa Sarai <cyphar-gVpy/LI/lHzQT0dZR+AlfA@public.gmane.org>
CC: "Amanieu d'Antras" <redacted>
CC: Paul Moore <redacted>
CC: Sasha Levin <redacted>
CC: Palmer Dabbelt <redacted>
CC: Vladimir Davydov <redacted>
CC: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Andy Lutomirski <redacted>

Signed-off-by: Stas Sergeev <redacted>
---
  include/linux/sched.h       |  8 ++++++++
  include/linux/signal.h      |  4 +++-
  include/uapi/linux/signal.h |  3 +++
  kernel/fork.c               |  2 +-
  kernel/signal.c             | 23 ++++++++++++-----------
  5 files changed, 27 insertions(+), 13 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 e1bd50c2..4691bc5 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h

@@ -6,5 +6,8 @@

  #define SS_ONSTACK     1
  #define SS_DISABLE     2
+#define SS_VALMASK     0xf

SS_MODE_MASK, perhaps?

Actually, let's invert that.

#define SS_AUTODISARM (1U << 31)
#define SS_FLAG_BITS SS_AUTODISARM

ss_mode = ss_flags & ~SS_FLAG_BITS;

this way flag bits that are currently undefined will continue to trigger
EINVAL.

OK, will do.
Though I am still going to keep SS_AUTODISARM =(1 << 4) -
it doesn't matter from what side we add new flags, does it?

Adding them starting with a high bit means that more low bits are
available for use as mode numbers.  Admittedly, this barely matters.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC