[PATCH 0/4] SS_AUTODISARM fixes and an ABI change

[PATCH 0/4] SS_AUTODISARM fixes and an ABI change

[Andy Lutomirski]

The first three are fixes IMO.  The fourth changes the SS_AUTODISARM
bit.  I'm assuming that's okay, as the bit has existed in -tip for
less than a day.

Andy Lutomirski (4):
  signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack
  selftests/sigaltstack: Fix the sas test on old kernels
  signals/sigaltstack: Report current flag bits in sigaltstack()
  signals/sigaltstack: Change SS_AUTODISARM to (1U << 31)

 include/linux/sched.h                     | 12 +++++++++
 include/uapi/linux/signal.h               |  2 +-
 kernel/signal.c                           |  3 ++-
 tools/testing/selftests/sigaltstack/sas.c | 42 +++++++++++++++++++++++--------
 4 files changed, 46 insertions(+), 13 deletions(-)

-- 
2.5.5

[PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack

[Andy Lutomirski]

If a signal stack is set up with SS_AUTODISARM, then the kernel
inherently avoids incorrectly resetting the signal stack if signals
recurse: the signal stack will be reset on the first signal
delivery.  This means that we don't need check the stack pointer
when delivering signals if SS_AUTODISARM is set.

This will make segmented x86 programs more robust: currently there's
a hole that could be triggered if ESP/RSP appears to point to the
signal stack but actually doesn't due to a nonzero SS base.

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
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 include/linux/sched.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 2950c5cd3005..8f03a93348b9 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2576,6 +2576,18 @@ static inline int kill_cad_pid(int sig, int priv)
  */
 static inline int on_sig_stack(unsigned long sp)
 {
+	/*
+	 * If the signal stack is AUTODISARM then, by construction, we
+	 * can't be on the signal stack unless user code deliberately set
+	 * SS_AUTODISARM when we were already on the it.
+	 *
+	 * This improve reliability: if user state gets corrupted such that
+	 * the stack pointer points very close to the end of the signal stack,
+	 * then this check will enable the signal to be handled anyway.
+	 */
+	if (current->sas_ss_flags & SS_AUTODISARM)
+		return 0;
+
 #ifdef CONFIG_STACK_GROWSUP
 	return sp >= current->sas_ss_sp &&
 		sp - current->sas_ss_sp < current->sas_ss_size;
-- 
2.5.5

[PATCH 2/4] selftests/sigaltstack: Fix the sas test on old kernels

[Andy Lutomirski]

The handling for old kernels was wrong.  Fix it.

Reported-by: Ingo Molnar <mingo@kernel.org>
Cc: 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
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 tools/testing/selftests/sigaltstack/sas.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
index 57da8bfde60b..a98c3ef8141f 100644
--- a/tools/testing/selftests/sigaltstack/sas.c
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -15,6 +15,7 @@
 #include <alloca.h>
 #include <string.h>
 #include <assert.h>
+#include <errno.h>
 
 #ifndef SS_AUTODISARM
 #define SS_AUTODISARM  (1 << 4)
@@ -117,13 +118,19 @@ int main(void)
 	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;
+		if (errno == EINVAL) {
+			printf("[NOTE]\tThe running kernel doesn't support SS_AUTODISARM\n");
+			/*
+			 * If test cases for the !SS_AUTODISARM variant were
+			 * added, we could still run them.  We don't have any
+			 * test cases like that yet, so just exit and report
+			 * success.
+			 */
+			return 0;
+		} else {
+			perror("[FAIL]\tsigaltstack(SS_ONSTACK | SS_AUTODISARM)");
+			return EXIT_FAILURE;
+		}
 	}
 
 	ustack = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
-- 
2.5.5

[PATCH 3/4] signals/sigaltstack: Report current flag bits in sigaltstack()

[Andy Lutomirski]

sigaltstack()'s reported previous state uses a somewhat odd
convention, but the concept of flag bits is new, and we can do the
flag bits sensibly.  Specifically, let's just report them directly.

This will allow saving and restoring the sigaltstack state using
sigaltstack() to work correctly.

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
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 kernel/signal.c                           |  3 ++-
 tools/testing/selftests/sigaltstack/sas.c | 19 ++++++++++++++++---
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index bf97ea5775ae..ab122a2cee41 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3099,7 +3099,8 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 
 	oss.ss_sp = (void __user *) current->sas_ss_sp;
 	oss.ss_size = current->sas_ss_size;
-	oss.ss_flags = sas_ss_flags(sp);
+	oss.ss_flags = sas_ss_flags(sp) |
+		(current->sas_ss_flags & SS_FLAG_BITS);
 
 	if (uss) {
 		void __user *ss_sp;
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
index a98c3ef8141f..4280d0699792 100644
--- a/tools/testing/selftests/sigaltstack/sas.c
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -113,6 +113,19 @@ int main(void)
 		perror("mmap()");
 		return EXIT_FAILURE;
 	}
+
+	err = sigaltstack(NULL, &stk);
+	if (err) {
+		perror("[FAIL]\tsigaltstack()");
+		exit(EXIT_FAILURE);
+	}
+	if (stk.ss_flags == SS_DISABLE) {
+		printf("[OK]\tInitial sigaltstack state was SS_DISABLE\n");
+	} else {
+		printf("[FAIL]\tInitial sigaltstack state was %i; should have been SS_DISABLE\n", stk.ss_flags);
+		return EXIT_FAILURE;
+	}
+
 	stk.ss_sp = sstack;
 	stk.ss_size = SIGSTKSZ;
 	stk.ss_flags = SS_ONSTACK | SS_AUTODISARM;
@@ -151,12 +164,12 @@ int main(void)
 		perror("[FAIL]\tsigaltstack()");
 		exit(EXIT_FAILURE);
 	}
-	if (stk.ss_flags != 0) {
-		printf("[FAIL]\tss_flags=%i, should be 0\n",
+	if (stk.ss_flags != SS_AUTODISARM) {
+		printf("[FAIL]\tss_flags=%i, should be SS_AUTODISARM\n",
 				stk.ss_flags);
 		exit(EXIT_FAILURE);
 	}
-	printf("[OK]\tsigaltstack is enabled after signal\n");
+	printf("[OK]\tsigaltstack is still SS_AUTODISARM after signal\n");
 
 	printf("[OK]\tTest passed\n");
 	return 0;
-- 
2.5.5

[PATCH 4/4] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31)

[Andy Lutomirski]

Using bit 4 divides the space of available bits strangely.  Use bit
31 instead so that we have a better chance of keeping flag and mode
bits separate in the long run.

Cc: 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
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 include/uapi/linux/signal.h               | 2 +-
 tools/testing/selftests/sigaltstack/sas.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index 738826048af2..cd0804b6bfa2 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -8,7 +8,7 @@
 #define SS_DISABLE	2
 
 /* bit-flags */
-#define SS_AUTODISARM	(1 << 4)	/* disable sas during sighandling */
+#define SS_AUTODISARM	(1U << 31)	/* disable sas during sighandling */
 /* mask for all SS_xxx flags */
 #define SS_FLAG_BITS	SS_AUTODISARM
 
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
index 4280d0699792..1bb01258e559 100644
--- a/tools/testing/selftests/sigaltstack/sas.c
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -18,7 +18,7 @@
 #include <errno.h>
 
 #ifndef SS_AUTODISARM
-#define SS_AUTODISARM  (1 << 4)
+#define SS_AUTODISARM  (1U << 31)
 #endif
 
 static void *sstack, *ustack;
-- 
2.5.5

Re: [PATCH 0/4] SS_AUTODISARM fixes and an ABI change

[Ingo Molnar]

* Andy Lutomirski <luto@kernel.org> wrote:

> The first three are fixes IMO.  The fourth changes the SS_AUTODISARM
> bit.  I'm assuming that's okay, as the bit has existed in -tip for
> less than a day.

Yeah, it's absolutely OK - I made it a standalone tree so we could even rebase it, 
but I think authorship and intent is clearer with your series on top of Stas's 
original commits.

Thanks,

	Ingo

Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack

[Ingo Molnar]

* Andy Lutomirski <luto@kernel.org> wrote:

> If a signal stack is set up with SS_AUTODISARM, then the kernel
> inherently avoids incorrectly resetting the signal stack if signals
> recurse: the signal stack will be reset on the first signal
> delivery.  This means that we don't need check the stack pointer
> when delivering signals if SS_AUTODISARM is set.
> 
> This will make segmented x86 programs more robust: currently there's
> a hole that could be triggered if ESP/RSP appears to point to the
> signal stack but actually doesn't due to a nonzero SS base.
> 
> Signed-off-by: Stas Sergeev <stsp@list.ru>

Presuably that SOB from Stas is stray, as there's no matching From: line?
I've removed it.

Thanks,

	Ingo

Re: [PATCH 3/4] signals/sigaltstack: Report current flag bits in sigaltstack()

[Ingo Molnar]

* Andy Lutomirski <luto@kernel.org> wrote:

> sigaltstack()'s reported previous state uses a somewhat odd
> convention, but the concept of flag bits is new, and we can do the
> flag bits sensibly.  Specifically, let's just report them directly.
> 
> This will allow saving and restoring the sigaltstack state using
> sigaltstack() to work correctly.
> 
> Signed-off-by: Stas Sergeev <stsp@list.ru>

This SOB seems stray as well, so I've removed it too.

Thanks,

	Ingo

[tip:core/signals] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack()

[tip-bot for Andy Lutomirski]

Commit-ID:  c876eeab6432687846d4cd5fe1e43dbc348de134
Gitweb:     http://git.kernel.org/tip/c876eeab6432687846d4cd5fe1e43dbc348de134
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Tue, 3 May 2016 10:31:49 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 4 May 2016 08:34:13 +0200

signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack()

If a signal stack is set up with SS_AUTODISARM, then the kernel
inherently avoids incorrectly resetting the signal stack if signals
recurse: the signal stack will be reset on the first signal
delivery.  This means that we don't need check the stack pointer
when delivering signals if SS_AUTODISARM is set.

This will make segmented x86 programs more robust: currently there's
a hole that could be triggered if ESP/RSP appears to point to the
signal stack but actually doesn't due to a nonzero SS base.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
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: Stas Sergeev <stsp@list.ru>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vladimir Davydov <vdavydov@parallels.com>
Cc: linux-api@vger.kernel.org
Link: http://lkml.kernel.org/r/c46bee4654ca9e68c498462fd11746e2bd0d98c8.1462296606.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/sched.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 2950c5c..77fd49f 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2576,6 +2576,18 @@ static inline int kill_cad_pid(int sig, int priv)
  */
 static inline int on_sig_stack(unsigned long sp)
 {
+	/*
+	 * If the signal stack is SS_AUTODISARM then, by construction, we
+	 * can't be on the signal stack unless user code deliberately set
+	 * SS_AUTODISARM when we were already on it.
+	 *
+	 * This improves reliability: if user state gets corrupted such that
+	 * the stack pointer points very close to the end of the signal stack,
+	 * then this check will enable the signal to be handled anyway.
+	 */
+	if (current->sas_ss_flags & SS_AUTODISARM)
+		return 0;
+
 #ifdef CONFIG_STACK_GROWSUP
 	return sp >= current->sas_ss_sp &&
 		sp - current->sas_ss_sp < current->sas_ss_size;

[tip:core/signals] selftests/sigaltstack: Fix the sigaltstack test on old kernels

[tip-bot for Andy Lutomirski]

Commit-ID:  158b67b5c5ccda9b909f18028a3cd17185ca1efd
Gitweb:     http://git.kernel.org/tip/158b67b5c5ccda9b909f18028a3cd17185ca1efd
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Tue, 3 May 2016 10:31:50 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 4 May 2016 08:34:13 +0200

selftests/sigaltstack: Fix the sigaltstack test on old kernels

The handling for old kernels was wrong, resulting in a segfault.  Fix it.

Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
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: Stas Sergeev <stsp@list.ru>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-api@vger.kernel.org
Link: http://lkml.kernel.org/r/f3e739bf435beeaecbd5f038f1359d2eac6d1e63.1462296606.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/testing/selftests/sigaltstack/sas.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
index 57da8bf..a98c3ef 100644
--- a/tools/testing/selftests/sigaltstack/sas.c
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -15,6 +15,7 @@
 #include <alloca.h>
 #include <string.h>
 #include <assert.h>
+#include <errno.h>
 
 #ifndef SS_AUTODISARM
 #define SS_AUTODISARM  (1 << 4)
@@ -117,13 +118,19 @@ int main(void)
 	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;
+		if (errno == EINVAL) {
+			printf("[NOTE]\tThe running kernel doesn't support SS_AUTODISARM\n");
+			/*
+			 * If test cases for the !SS_AUTODISARM variant were
+			 * added, we could still run them.  We don't have any
+			 * test cases like that yet, so just exit and report
+			 * success.
+			 */
+			return 0;
+		} else {
+			perror("[FAIL]\tsigaltstack(SS_ONSTACK | SS_AUTODISARM)");
+			return EXIT_FAILURE;
+		}
 	}
 
 	ustack = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,

[tip:core/signals] signals/sigaltstack: Report current flag bits in sigaltstack()

[tip-bot for Andy Lutomirski]

Commit-ID:  0318bc8a919ded355eaa5078689924a15c1bf52a
Gitweb:     http://git.kernel.org/tip/0318bc8a919ded355eaa5078689924a15c1bf52a
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Tue, 3 May 2016 10:31:51 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 4 May 2016 08:34:14 +0200

signals/sigaltstack: Report current flag bits in sigaltstack()

sigaltstack()'s reported previous state uses a somewhat odd
convention, but the concept of flag bits is new, and we can do the
flag bits sensibly.  Specifically, let's just report them directly.

This will allow saving and restoring the sigaltstack state using
sigaltstack() to work correctly.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
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: Stas Sergeev <stsp@list.ru>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vladimir Davydov <vdavydov@parallels.com>
Cc: linux-api@vger.kernel.org
Link: http://lkml.kernel.org/r/94b291ec9fd47741a9264851e316e158ded0b00d.1462296606.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/signal.c                           |  3 ++-
 tools/testing/selftests/sigaltstack/sas.c | 19 ++++++++++++++++---
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index bf97ea5..ab122a2 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3099,7 +3099,8 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
 
 	oss.ss_sp = (void __user *) current->sas_ss_sp;
 	oss.ss_size = current->sas_ss_size;
-	oss.ss_flags = sas_ss_flags(sp);
+	oss.ss_flags = sas_ss_flags(sp) |
+		(current->sas_ss_flags & SS_FLAG_BITS);
 
 	if (uss) {
 		void __user *ss_sp;
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
index a98c3ef..4280d06 100644
--- a/tools/testing/selftests/sigaltstack/sas.c
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -113,6 +113,19 @@ int main(void)
 		perror("mmap()");
 		return EXIT_FAILURE;
 	}
+
+	err = sigaltstack(NULL, &stk);
+	if (err) {
+		perror("[FAIL]\tsigaltstack()");
+		exit(EXIT_FAILURE);
+	}
+	if (stk.ss_flags == SS_DISABLE) {
+		printf("[OK]\tInitial sigaltstack state was SS_DISABLE\n");
+	} else {
+		printf("[FAIL]\tInitial sigaltstack state was %i; should have been SS_DISABLE\n", stk.ss_flags);
+		return EXIT_FAILURE;
+	}
+
 	stk.ss_sp = sstack;
 	stk.ss_size = SIGSTKSZ;
 	stk.ss_flags = SS_ONSTACK | SS_AUTODISARM;
@@ -151,12 +164,12 @@ int main(void)
 		perror("[FAIL]\tsigaltstack()");
 		exit(EXIT_FAILURE);
 	}
-	if (stk.ss_flags != 0) {
-		printf("[FAIL]\tss_flags=%i, should be 0\n",
+	if (stk.ss_flags != SS_AUTODISARM) {
+		printf("[FAIL]\tss_flags=%i, should be SS_AUTODISARM\n",
 				stk.ss_flags);
 		exit(EXIT_FAILURE);
 	}
-	printf("[OK]\tsigaltstack is enabled after signal\n");
+	printf("[OK]\tsigaltstack is still SS_AUTODISARM after signal\n");
 
 	printf("[OK]\tTest passed\n");
 	return 0;

[tip:core/signals] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31)

[tip-bot for Andy Lutomirski]

Commit-ID:  91c6180572e2fec71701d646ffc40ad30986275c
Gitweb:     http://git.kernel.org/tip/91c6180572e2fec71701d646ffc40ad30986275c
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Tue, 3 May 2016 10:31:52 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 4 May 2016 08:34:14 +0200

signals/sigaltstack: Change SS_AUTODISARM to (1U << 31)

Using bit 4 divides the space of available bits strangely.  Use bit
31 instead so that we have a better chance of keeping flag and mode
bits separate in the long run.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
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: Stas Sergeev <stsp@list.ru>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vladimir Davydov <vdavydov@parallels.com>
Cc: linux-api@vger.kernel.org
Link: http://lkml.kernel.org/r/bb996508a600af14b406810c3d58fe0e0d0afe0d.1462296606.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/uapi/linux/signal.h               | 2 +-
 tools/testing/selftests/sigaltstack/sas.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index 7388260..cd0804b 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -8,7 +8,7 @@
 #define SS_DISABLE	2
 
 /* bit-flags */
-#define SS_AUTODISARM	(1 << 4)	/* disable sas during sighandling */
+#define SS_AUTODISARM	(1U << 31)	/* disable sas during sighandling */
 /* mask for all SS_xxx flags */
 #define SS_FLAG_BITS	SS_AUTODISARM
 
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
index 4280d06..1bb0125 100644
--- a/tools/testing/selftests/sigaltstack/sas.c
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -18,7 +18,7 @@
 #include <errno.h>
 
 #ifndef SS_AUTODISARM
-#define SS_AUTODISARM  (1 << 4)
+#define SS_AUTODISARM  (1U << 31)
 #endif
 
 static void *sstack, *ustack;

Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack

[Andy Lutomirski]

On May 3, 2016 11:32 PM, "Ingo Molnar" <mingo@kernel.org> wrote:
>
>
> * Andy Lutomirski <luto@kernel.org> wrote:
>
> > If a signal stack is set up with SS_AUTODISARM, then the kernel
> > inherently avoids incorrectly resetting the signal stack if signals
> > recurse: the signal stack will be reset on the first signal
> > delivery.  This means that we don't need check the stack pointer
> > when delivering signals if SS_AUTODISARM is set.
> >
> > This will make segmented x86 programs more robust: currently there's
> > a hole that could be triggered if ESP/RSP appears to point to the
> > signal stack but actually doesn't due to a nonzero SS base.
> >
> > Signed-off-by: Stas Sergeev <stsp@list.ru>
>
> Presuably that SOB from Stas is stray, as there's no matching From: line?
> I've removed it.

Yes.  It was a cut-and-paste-o -- I meant to change it to cc.

>
> Thanks,
>
>         Ingo

Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack

[Stas Sergeev]

03.05.2016 20:31, Andy Lutomirski пишет:
> If a signal stack is set up with SS_AUTODISARM, then the kernel
> inherently avoids incorrectly resetting the signal stack if signals
> recurse: the signal stack will be reset on the first signal
> delivery.  This means that we don't need check the stack pointer
> when delivering signals if SS_AUTODISARM is set.
>
> This will make segmented x86 programs more robust: currently there's
> a hole that could be triggered if ESP/RSP appears to point to the
> signal stack but actually doesn't due to a nonzero SS base.
>
> 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
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>   include/linux/sched.h | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 2950c5cd3005..8f03a93348b9 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2576,6 +2576,18 @@ static inline int kill_cad_pid(int sig, int priv)
>    */
>   static inline int on_sig_stack(unsigned long sp)
>   {
> +	/*
> +	 * If the signal stack is AUTODISARM then, by construction, we
> +	 * can't be on the signal stack unless user code deliberately set
> +	 * SS_AUTODISARM when we were already on the it.
"on the it" -> "on it".

Anyway, I am a bit puzzled with this patch.
You say "unless user code deliberately set
SS_AUTODISARM when we were already on the it"
so what happens in case it actually does?

Without your patch: if user sets up the same sas - no stack switch.
if user sets up different sas - stack switch on nested signal.

With your patch: stack switch in any case, so if user
set up same sas - stack corruption by nested signal.

Or am I missing the intention?

Re: [PATCH 2/4] selftests/sigaltstack: Fix the sas test on old kernels

[Stas Sergeev]

03.05.2016 20:31, Andy Lutomirski пишет:
> The handling for old kernels was wrong.  Fix it.
>
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Cc: 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
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>   tools/testing/selftests/sigaltstack/sas.c | 21 ++++++++++++++-------
>   1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
> index 57da8bfde60b..a98c3ef8141f 100644
> --- a/tools/testing/selftests/sigaltstack/sas.c
> +++ b/tools/testing/selftests/sigaltstack/sas.c
> @@ -15,6 +15,7 @@
>   #include <alloca.h>
>   #include <string.h>
>   #include <assert.h>
> +#include <errno.h>
>   
>   #ifndef SS_AUTODISARM
>   #define SS_AUTODISARM  (1 << 4)
> @@ -117,13 +118,19 @@ int main(void)
>   	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;
> +		if (errno == EINVAL) {
> +			printf("[NOTE]\tThe running kernel doesn't support SS_AUTODISARM\n");
> +			/*
> +			 * If test cases for the !SS_AUTODISARM variant were
> +			 * added, we could still run them.  We don't have any
> +			 * test cases like that yet, so just exit and report
> +			 * success.
> +			 */
But that was the point, please see how it handles the
old kernels:

$ ./sas
[FAIL]    sigaltstack(SS_ONSTACK | SS_AUTODISARM): Invalid argument
[RUN]    signal USR1
[FAIL]    ss_flags=1, should be SS_DISABLE
[RUN]    switched to user ctx
[RUN]    signal USR2
[FAIL]    sigaltstack re-used
[FAIL]    Stack corrupted
[RUN]    Aborting

Unfortunalely, for Ingo it crashed...
I am not sure why, I can't reproduce. :(
So if you disable all the "old" tests, you can as well
remove them, or... find the bug and re-enable. :)

Re: [PATCH 4/4] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31)

[Stas Sergeev]

03.05.2016 20:31, Andy Lutomirski пишет:
> Using bit 4 divides the space of available bits strangely.  Use bit
> 31 instead so that we have a better chance of keeping flag and mode
> bits separate in the long run.
>
> Cc: 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
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>   include/uapi/linux/signal.h               | 2 +-
>   tools/testing/selftests/sigaltstack/sas.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
> index 738826048af2..cd0804b6bfa2 100644
> --- a/include/uapi/linux/signal.h
> +++ b/include/uapi/linux/signal.h
> @@ -8,7 +8,7 @@
>   #define SS_DISABLE	2
>   
>   /* bit-flags */
> -#define SS_AUTODISARM	(1 << 4)	/* disable sas during sighandling */
> +#define SS_AUTODISARM	(1U << 31)	/* disable sas during sighandling */
And what if we are out of 32 bits for storing both mode and flags? :)
Well, yes, very unlikely, but I did it that way exactly so that
we can eventually promote to 64bit variable.
Doesn't matter at all, of course. Let it be any way you like.

Re: [PATCH 2/4] selftests/sigaltstack: Fix the sas test on old kernels

[Andy Lutomirski]

On May 7, 2016 8:02 AM, "Stas Sergeev" <stsp@list.ru> wrote:
>
> 03.05.2016 20:31, Andy Lutomirski пишет:
>
>> The handling for old kernels was wrong.  Fix it.
>>
>> Reported-by: Ingo Molnar <mingo@kernel.org>
>> Cc: 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
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>   tools/testing/selftests/sigaltstack/sas.c | 21 ++++++++++++++-------
>>   1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
>> index 57da8bfde60b..a98c3ef8141f 100644
>> --- a/tools/testing/selftests/sigaltstack/sas.c
>> +++ b/tools/testing/selftests/sigaltstack/sas.c
>> @@ -15,6 +15,7 @@
>>   #include <alloca.h>
>>   #include <string.h>
>>   #include <assert.h>
>> +#include <errno.h>
>>     #ifndef SS_AUTODISARM
>>   #define SS_AUTODISARM  (1 << 4)
>> @@ -117,13 +118,19 @@ int main(void)
>>         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;
>> +               if (errno == EINVAL) {
>> +                       printf("[NOTE]\tThe running kernel doesn't support SS_AUTODISARM\n");
>> +                       /*
>> +                        * If test cases for the !SS_AUTODISARM variant were
>> +                        * added, we could still run them.  We don't have any
>> +                        * test cases like that yet, so just exit and report
>> +                        * success.
>> +                        */
>
> But that was the point, please see how it handles the
> old kernels:
>
> $ ./sas
> [FAIL]    sigaltstack(SS_ONSTACK | SS_AUTODISARM): Invalid argument
> [RUN]    signal USR1
> [FAIL]    ss_flags=1, should be SS_DISABLE
> [RUN]    switched to user ctx
> [RUN]    signal USR2
> [FAIL]    sigaltstack re-used
> [FAIL]    Stack corrupted
> [RUN]    Aborting

This is useful as a demonstration of why the feature is useful, but it
doesn't indicate that anything is wrong with old kernels per she.
That's why I changed it to simply report that the feature is missing.

Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack

[Andy Lutomirski]

On May 7, 2016 7:38 AM, "Stas Sergeev" <stsp@list.ru> wrote:
>
> 03.05.2016 20:31, Andy Lutomirski пишет:
>
>> If a signal stack is set up with SS_AUTODISARM, then the kernel
>> inherently avoids incorrectly resetting the signal stack if signals
>> recurse: the signal stack will be reset on the first signal
>> delivery.  This means that we don't need check the stack pointer
>> when delivering signals if SS_AUTODISARM is set.
>>
>> This will make segmented x86 programs more robust: currently there's
>> a hole that could be triggered if ESP/RSP appears to point to the
>> signal stack but actually doesn't due to a nonzero SS base.
>>
>> 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
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>   include/linux/sched.h | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 2950c5cd3005..8f03a93348b9 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -2576,6 +2576,18 @@ static inline int kill_cad_pid(int sig, int priv)
>>    */
>>   static inline int on_sig_stack(unsigned long sp)
>>   {
>> +       /*
>> +        * If the signal stack is AUTODISARM then, by construction, we
>> +        * can't be on the signal stack unless user code deliberately set
>> +        * SS_AUTODISARM when we were already on the it.
>
> "on the it" -> "on it".
>
> Anyway, I am a bit puzzled with this patch.
> You say "unless user code deliberately set
>
> SS_AUTODISARM when we were already on the it"
> so what happens in case it actually does?
>

Stack corruption.  Don't do that.

> Without your patch: if user sets up the same sas - no stack switch.
> if user sets up different sas - stack switch on nested signal.
>
> With your patch: stack switch in any case, so if user
> set up same sas - stack corruption by nested signal.
>
> Or am I missing the intention?

The intention is to make everything completely explicit.  With
SS_AUTODISARM, the kernel knows directly whether you're on the signal
stack, and there should be no need to look at sp.  If you set
SS_AUTODISARM and get a signal, the signal stack gets disarmed.  If
you take a nested signal, it's delivered normally.  When you return
all the way out, the signal stack is re-armed.

For DOSEMU, this means that no 16-bit register state can possibly
cause a signal to be delivered wrong, because the register state when
a signal is raised won't affect delivery, which seems like a good
thing to me.

If this behavior would be problematic for you, can you explain why?

Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack

[Stas Sergeev]

09.05.2016 04:32, Andy Lutomirski пишет:
> On May 7, 2016 7:38 AM, "Stas Sergeev" <stsp@list.ru> wrote:
>> 03.05.2016 20:31, Andy Lutomirski пишет:
>>
>>> If a signal stack is set up with SS_AUTODISARM, then the kernel
>>> inherently avoids incorrectly resetting the signal stack if signals
>>> recurse: the signal stack will be reset on the first signal
>>> delivery.  This means that we don't need check the stack pointer
>>> when delivering signals if SS_AUTODISARM is set.
>>>
>>> This will make segmented x86 programs more robust: currently there's
>>> a hole that could be triggered if ESP/RSP appears to point to the
>>> signal stack but actually doesn't due to a nonzero SS base.
>>>
>>> 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
>>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>>> ---
>>>    include/linux/sched.h | 12 ++++++++++++
>>>    1 file changed, 12 insertions(+)
>>>
>>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>>> index 2950c5cd3005..8f03a93348b9 100644
>>> --- a/include/linux/sched.h
>>> +++ b/include/linux/sched.h
>>> @@ -2576,6 +2576,18 @@ static inline int kill_cad_pid(int sig, int priv)
>>>     */
>>>    static inline int on_sig_stack(unsigned long sp)
>>>    {
>>> +       /*
>>> +        * If the signal stack is AUTODISARM then, by construction, we
>>> +        * can't be on the signal stack unless user code deliberately set
>>> +        * SS_AUTODISARM when we were already on the it.
>> "on the it" -> "on it".
>>
>> Anyway, I am a bit puzzled with this patch.
>> You say "unless user code deliberately set
>>
>> SS_AUTODISARM when we were already on the it"
>> so what happens in case it actually does?
>>
> Stack corruption.  Don't do that.
Only after your change, I have to admit. :)

>> Without your patch: if user sets up the same sas - no stack switch.
>> if user sets up different sas - stack switch on nested signal.
>>
>> With your patch: stack switch in any case, so if user
>> set up same sas - stack corruption by nested signal.
>>
>> Or am I missing the intention?
> The intention is to make everything completely explicit.  With
> SS_AUTODISARM, the kernel knows directly whether you're on the signal
> stack, and there should be no need to look at sp.  If you set
> SS_AUTODISARM and get a signal, the signal stack gets disarmed.  If
> you take a nested signal, it's delivered normally.  When you return
> all the way out, the signal stack is re-armed.
>
> For DOSEMU, this means that no 16-bit register state can possibly
> cause a signal to be delivered wrong, because the register state when
> a signal is raised won't affect delivery, which seems like a good
> thing to me.
Yes, but doesn't affect dosemu1 which doesn't use SS_AUTODISARM.
So IMHO the SS check should still be added, even if not for dosemu2.

> If this behavior would be problematic for you, can you explain why?
Only theoretically: if someone sets SS_AUTODISARM inside a
sighandler. Since this doesn't give EPERM, I wouldn't deliberately
make it a broken scenario (esp if it wasn't before the particular change).
Ideally it would give EPERM, but we can't, so doesn't matter much.
I just wanted to warn about the possible regression.

Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack

[Andy Lutomirski]

On May 8, 2016 7:05 PM, "Stas Sergeev" <stsp@list.ru> wrote:
>
> 09.05.2016 04:32, Andy Lutomirski пишет:
>
>> On May 7, 2016 7:38 AM, "Stas Sergeev" <stsp@list.ru> wrote:
>>>
>>> 03.05.2016 20:31, Andy Lutomirski пишет:
>>>
>>>> If a signal stack is set up with SS_AUTODISARM, then the kernel
>>>> inherently avoids incorrectly resetting the signal stack if signals
>>>> recurse: the signal stack will be reset on the first signal
>>>> delivery.  This means that we don't need check the stack pointer
>>>> when delivering signals if SS_AUTODISARM is set.
>>>>
>>>> This will make segmented x86 programs more robust: currently there's
>>>> a hole that could be triggered if ESP/RSP appears to point to the
>>>> signal stack but actually doesn't due to a nonzero SS base.
>>>>
>>>> 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
>>>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>>>> ---
>>>>    include/linux/sched.h | 12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>>>> index 2950c5cd3005..8f03a93348b9 100644
>>>> --- a/include/linux/sched.h
>>>> +++ b/include/linux/sched.h
>>>> @@ -2576,6 +2576,18 @@ static inline int kill_cad_pid(int sig, int priv)
>>>>     */
>>>>    static inline int on_sig_stack(unsigned long sp)
>>>>    {
>>>> +       /*
>>>> +        * If the signal stack is AUTODISARM then, by construction, we
>>>> +        * can't be on the signal stack unless user code deliberately set
>>>> +        * SS_AUTODISARM when we were already on the it.
>>>
>>> "on the it" -> "on it".
>>>
>>> Anyway, I am a bit puzzled with this patch.
>>> You say "unless user code deliberately set
>>>
>>> SS_AUTODISARM when we were already on the it"
>>> so what happens in case it actually does?
>>>
>> Stack corruption.  Don't do that.
>
> Only after your change, I have to admit. :)
>
>
>>> Without your patch: if user sets up the same sas - no stack switch.
>>> if user sets up different sas - stack switch on nested signal.
>>>
>>> With your patch: stack switch in any case, so if user
>>> set up same sas - stack corruption by nested signal.
>>>
>>> Or am I missing the intention?
>>
>> The intention is to make everything completely explicit.  With
>> SS_AUTODISARM, the kernel knows directly whether you're on the signal
>> stack, and there should be no need to look at sp.  If you set
>> SS_AUTODISARM and get a signal, the signal stack gets disarmed.  If
>> you take a nested signal, it's delivered normally.  When you return
>> all the way out, the signal stack is re-armed.
>>
>> For DOSEMU, this means that no 16-bit register state can possibly
>> cause a signal to be delivered wrong, because the register state when
>> a signal is raised won't affect delivery, which seems like a good
>> thing to me.
>
> Yes, but doesn't affect dosemu1 which doesn't use SS_AUTODISARM.
> So IMHO the SS check should still be added, even if not for dosemu2.
>
>
>> If this behavior would be problematic for you, can you explain why?
>
> Only theoretically: if someone sets SS_AUTODISARM inside a
> sighandler. Since this doesn't give EPERM, I wouldn't deliberately
> make it a broken scenario (esp if it wasn't before the particular change).
> Ideally it would give EPERM, but we can't, so doesn't matter much.
> I just wanted to warn about the possible regression.

I suppose we could return an error if you are on the sigstack when
setting SS_AUTODISARM, although I was hoping to avoid yet more special
cases.

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

Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack

[Stas Sergeev]

14.05.2016 07:18, Andy Lutomirski пишет:
> On May 8, 2016 7:05 PM, "Stas Sergeev" <stsp@list.ru> wrote:
>> 09.05.2016 04:32, Andy Lutomirski пишет:
>>
>>> On May 7, 2016 7:38 AM, "Stas Sergeev" <stsp@list.ru> wrote:
>>>> 03.05.2016 20:31, Andy Lutomirski пишет:
>>>>
>>>>> If a signal stack is set up with SS_AUTODISARM, then the kernel
>>>>> inherently avoids incorrectly resetting the signal stack if signals
>>>>> recurse: the signal stack will be reset on the first signal
>>>>> delivery.  This means that we don't need check the stack pointer
>>>>> when delivering signals if SS_AUTODISARM is set.
>>>>>
>>>>> This will make segmented x86 programs more robust: currently there's
>>>>> a hole that could be triggered if ESP/RSP appears to point to the
>>>>> signal stack but actually doesn't due to a nonzero SS base.
>>>>>
>>>>> 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
>>>>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>>>>> ---
>>>>>     include/linux/sched.h | 12 ++++++++++++
>>>>>     1 file changed, 12 insertions(+)
>>>>>
>>>>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>>>>> index 2950c5cd3005..8f03a93348b9 100644
>>>>> --- a/include/linux/sched.h
>>>>> +++ b/include/linux/sched.h
>>>>> @@ -2576,6 +2576,18 @@ static inline int kill_cad_pid(int sig, int priv)
>>>>>      */
>>>>>     static inline int on_sig_stack(unsigned long sp)
>>>>>     {
>>>>> +       /*
>>>>> +        * If the signal stack is AUTODISARM then, by construction, we
>>>>> +        * can't be on the signal stack unless user code deliberately set
>>>>> +        * SS_AUTODISARM when we were already on the it.
>>>> "on the it" -> "on it".
>>>>
>>>> Anyway, I am a bit puzzled with this patch.
>>>> You say "unless user code deliberately set
>>>>
>>>> SS_AUTODISARM when we were already on the it"
>>>> so what happens in case it actually does?
>>>>
>>> Stack corruption.  Don't do that.
>> Only after your change, I have to admit. :)
>>
>>
>>>> Without your patch: if user sets up the same sas - no stack switch.
>>>> if user sets up different sas - stack switch on nested signal.
>>>>
>>>> With your patch: stack switch in any case, so if user
>>>> set up same sas - stack corruption by nested signal.
>>>>
>>>> Or am I missing the intention?
>>> The intention is to make everything completely explicit.  With
>>> SS_AUTODISARM, the kernel knows directly whether you're on the signal
>>> stack, and there should be no need to look at sp.  If you set
>>> SS_AUTODISARM and get a signal, the signal stack gets disarmed.  If
>>> you take a nested signal, it's delivered normally.  When you return
>>> all the way out, the signal stack is re-armed.
>>>
>>> For DOSEMU, this means that no 16-bit register state can possibly
>>> cause a signal to be delivered wrong, because the register state when
>>> a signal is raised won't affect delivery, which seems like a good
>>> thing to me.
>> Yes, but doesn't affect dosemu1 which doesn't use SS_AUTODISARM.
>> So IMHO the SS check should still be added, even if not for dosemu2.
>>
>>
>>> If this behavior would be problematic for you, can you explain why?
>> Only theoretically: if someone sets SS_AUTODISARM inside a
>> sighandler. Since this doesn't give EPERM, I wouldn't deliberately
>> make it a broken scenario (esp if it wasn't before the particular change).
>> Ideally it would give EPERM, but we can't, so doesn't matter much.
>> I just wanted to warn about the possible regression.
> I suppose we could return an error if you are on the sigstack when
> setting SS_AUTODISARM, although I was hoping to avoid yet more special
> cases.
Hmm.
How about extending the generic check then?
Currently it is roughly:
if (on_sig_stack(sp)) return -EPERM;

and we could do:
if (on_sig_stack(sp) || on_new_sas(new_sas, sp)) return -EPERM;

Looks like it will close the potential hole opened by your commit
without introducing the special case for SS_AUTODISARM.
What do you think?

Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack

[Andy Lutomirski]

On May 14, 2016 4:18 AM, "Stas Sergeev" <stsp@list.ru> wrote:
>
> 14.05.2016 07:18, Andy Lutomirski пишет:
>
>> On May 8, 2016 7:05 PM, "Stas Sergeev" <stsp@list.ru> wrote:
>>>
>>> 09.05.2016 04:32, Andy Lutomirski пишет:
>>>
>>>> On May 7, 2016 7:38 AM, "Stas Sergeev" <stsp@list.ru> wrote:
>>>>>
>>>>> 03.05.2016 20:31, Andy Lutomirski пишет:
>>>>>
>>>>>> If a signal stack is set up with SS_AUTODISARM, then the kernel
>>>>>> inherently avoids incorrectly resetting the signal stack if signals
>>>>>> recurse: the signal stack will be reset on the first signal
>>>>>> delivery.  This means that we don't need check the stack pointer
>>>>>> when delivering signals if SS_AUTODISARM is set.
>>>>>>
>>>>>> This will make segmented x86 programs more robust: currently there's
>>>>>> a hole that could be triggered if ESP/RSP appears to point to the
>>>>>> signal stack but actually doesn't due to a nonzero SS base.
>>>>>>
>>>>>> 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
>>>>>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>>>>>> ---
>>>>>>     include/linux/sched.h | 12 ++++++++++++
>>>>>>     1 file changed, 12 insertions(+)
>>>>>>
>>>>>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>>>>>> index 2950c5cd3005..8f03a93348b9 100644
>>>>>> --- a/include/linux/sched.h
>>>>>> +++ b/include/linux/sched.h
>>>>>> @@ -2576,6 +2576,18 @@ static inline int kill_cad_pid(int sig, int priv)
>>>>>>      */
>>>>>>     static inline int on_sig_stack(unsigned long sp)
>>>>>>     {
>>>>>> +       /*
>>>>>> +        * If the signal stack is AUTODISARM then, by construction, we
>>>>>> +        * can't be on the signal stack unless user code deliberately set
>>>>>> +        * SS_AUTODISARM when we were already on the it.
>>>>>
>>>>> "on the it" -> "on it".
>>>>>
>>>>> Anyway, I am a bit puzzled with this patch.
>>>>> You say "unless user code deliberately set
>>>>>
>>>>> SS_AUTODISARM when we were already on the it"
>>>>> so what happens in case it actually does?
>>>>>
>>>> Stack corruption.  Don't do that.
>>>
>>> Only after your change, I have to admit. :)
>>>
>>>
>>>>> Without your patch: if user sets up the same sas - no stack switch.
>>>>> if user sets up different sas - stack switch on nested signal.
>>>>>
>>>>> With your patch: stack switch in any case, so if user
>>>>> set up same sas - stack corruption by nested signal.
>>>>>
>>>>> Or am I missing the intention?
>>>>
>>>> The intention is to make everything completely explicit.  With
>>>> SS_AUTODISARM, the kernel knows directly whether you're on the signal
>>>> stack, and there should be no need to look at sp.  If you set
>>>> SS_AUTODISARM and get a signal, the signal stack gets disarmed.  If
>>>> you take a nested signal, it's delivered normally.  When you return
>>>> all the way out, the signal stack is re-armed.
>>>>
>>>> For DOSEMU, this means that no 16-bit register state can possibly
>>>> cause a signal to be delivered wrong, because the register state when
>>>> a signal is raised won't affect delivery, which seems like a good
>>>> thing to me.
>>>
>>> Yes, but doesn't affect dosemu1 which doesn't use SS_AUTODISARM.
>>> So IMHO the SS check should still be added, even if not for dosemu2.
>>>
>>>
>>>> If this behavior would be problematic for you, can you explain why?
>>>
>>> Only theoretically: if someone sets SS_AUTODISARM inside a
>>> sighandler. Since this doesn't give EPERM, I wouldn't deliberately
>>> make it a broken scenario (esp if it wasn't before the particular change).
>>> Ideally it would give EPERM, but we can't, so doesn't matter much.
>>> I just wanted to warn about the possible regression.
>>
>> I suppose we could return an error if you are on the sigstack when
>> setting SS_AUTODISARM, although I was hoping to avoid yet more special
>> cases.
>
> Hmm.
> How about extending the generic check then?
> Currently it is roughly:
> if (on_sig_stack(sp)) return -EPERM;
>
> and we could do:
> if (on_sig_stack(sp) || on_new_sas(new_sas, sp)) return -EPERM;
>
> Looks like it will close the potential hole opened by your commit
> without introducing the special case for SS_AUTODISARM.
> What do you think?
>

It's still a wee bit ugly.  Also, doesn't that change existing
behavior for the existing non-AUTODISARM case?  Also, we'd have to
make sure that sigreturn doesn't trigger this check.

My inclination would be leave it alone.