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.
Change since v5:
- Fix description of patch 4/4
Change since v4:
- Implement this Andy Lutomirski's suggestion:
https://lkml.org/lkml/2016/3/6/158
that allows the run-time probing of the existence of the
flags added in the future.
[PATCH 1/4] [Cleanup] x86: signal: unify the sigaltstack check with
A clean-up patch that unifies x86's sigaltstack handling with
other arches.
[PATCH 2/4] sigaltstack: preparations for adding new SS_xxx flags
Andy's suggested changes
[PATCH 3/4] sigaltstack: implement SS_AUTODISARM flag
This patch implements SS_AUTODISARM flag
[PATCH 4/4] selftests: Add test for
This patch adds the selftest code for new functionality
CC: [email protected]
CC: [email protected]
CC: Andy Lutomirski <[email protected]>
CC: Oleg Nesterov <[email protected]>
CC: Shuah Khan <[email protected]>
CC: Ingo Molnar <[email protected]>
Diffstat:
arch/x86/kernel/signal.c | 23 +--
include/linux/sched.h | 8 +
include/linux/signal.h | 4
include/uapi/linux/signal.h | 7 +
kernel/fork.c | 2
kernel/signal.c | 26 ++--
tools/testing/selftests/Makefile | 1
tools/testing/selftests/sigaltstack/Makefile | 8 +
tools/testing/selftests/sigaltstack/sas.c | 156 +++++++++++++++++++++++++++
9 files changed, 208 insertions(+), 27 deletions(-)
This patch adds the test case for SS_AUTODISARM flag.
The test-case tries to set SS_AUTODISARM flag and checks if
the nested signal corrupts the stack after swapcontext().
CC: Shuah Khan <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Andy Lutomirski <[email protected]>
Signed-off-by: Stas Sergeev <[email protected]>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/sigaltstack/Makefile | 8 ++
tools/testing/selftests/sigaltstack/sas.c | 156 +++++++++++++++++++++++++++
3 files changed, 165 insertions(+)
create mode 100644 tools/testing/selftests/sigaltstack/Makefile
create mode 100644 tools/testing/selftests/sigaltstack/sas.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index b04afc3..ff9e5f2 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -19,6 +19,7 @@ TARGETS += powerpc
TARGETS += pstore
TARGETS += ptrace
TARGETS += seccomp
+TARGETS += sigaltstack
TARGETS += size
TARGETS += static_keys
TARGETS += sysctl
diff --git a/tools/testing/selftests/sigaltstack/Makefile b/tools/testing/selftests/sigaltstack/Makefile
new file mode 100644
index 0000000..56af56e
--- /dev/null
+++ b/tools/testing/selftests/sigaltstack/Makefile
@@ -0,0 +1,8 @@
+CFLAGS = -Wall
+BINARIES = sas
+all: $(BINARIES)
+
+include ../lib.mk
+
+clean:
+ rm -rf $(BINARIES)
diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
new file mode 100644
index 0000000..57da8bf
--- /dev/null
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -0,0 +1,156 @@
+/*
+ * Stas Sergeev <[email protected]>
+ *
+ * test sigaltstack(SS_ONSTACK | SS_AUTODISARM)
+ * If that succeeds, then swapcontext() can be used inside sighandler safely.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+#include <ucontext.h>
+#include <alloca.h>
+#include <string.h>
+#include <assert.h>
+
+#ifndef SS_AUTODISARM
+#define SS_AUTODISARM (1 << 4)
+#endif
+
+static void *sstack, *ustack;
+static ucontext_t uc, sc;
+static const char *msg = "[OK]\tStack preserved";
+static const char *msg2 = "[FAIL]\tStack corrupted";
+struct stk_data {
+ char msg[128];
+ int flag;
+};
+
+void my_usr1(int sig, siginfo_t *si, void *u)
+{
+ char *aa;
+ int err;
+ stack_t stk;
+ struct stk_data *p;
+
+ register unsigned long sp asm("sp");
+
+ if (sp < (unsigned long)sstack ||
+ sp >= (unsigned long)sstack + SIGSTKSZ) {
+ printf("[FAIL]\tSP is not on sigaltstack\n");
+ exit(EXIT_FAILURE);
+ }
+ /* put some data on stack. other sighandler will try to overwrite it */
+ aa = alloca(1024);
+ assert(aa);
+ p = (struct stk_data *)(aa + 512);
+ strcpy(p->msg, msg);
+ p->flag = 1;
+ printf("[RUN]\tsignal USR1\n");
+ err = sigaltstack(NULL, &stk);
+ if (err) {
+ perror("[FAIL]\tsigaltstack()");
+ exit(EXIT_FAILURE);
+ }
+ if (stk.ss_flags != SS_DISABLE)
+ printf("[FAIL]\tss_flags=%i, should be SS_DISABLE\n",
+ stk.ss_flags);
+ else
+ printf("[OK]\tsigaltstack is disabled in sighandler\n");
+ swapcontext(&sc, &uc);
+ printf("%s\n", p->msg);
+ if (!p->flag) {
+ printf("[RUN]\tAborting\n");
+ exit(EXIT_FAILURE);
+ }
+}
+
+void my_usr2(int sig, siginfo_t *si, void *u)
+{
+ char *aa;
+ struct stk_data *p;
+
+ printf("[RUN]\tsignal USR2\n");
+ aa = alloca(1024);
+ /* dont run valgrind on this */
+ /* try to find the data stored by previous sighandler */
+ p = memmem(aa, 1024, msg, strlen(msg));
+ if (p) {
+ printf("[FAIL]\tsigaltstack re-used\n");
+ /* corrupt the data */
+ strcpy(p->msg, msg2);
+ /* tell other sighandler that his data is corrupted */
+ p->flag = 0;
+ }
+}
+
+static void switch_fn(void)
+{
+ printf("[RUN]\tswitched to user ctx\n");
+ raise(SIGUSR2);
+ setcontext(&sc);
+}
+
+int main(void)
+{
+ struct sigaction act;
+ stack_t stk;
+ int err;
+
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = SA_ONSTACK | SA_SIGINFO;
+ act.sa_sigaction = my_usr1;
+ sigaction(SIGUSR1, &act, NULL);
+ act.sa_sigaction = my_usr2;
+ sigaction(SIGUSR2, &act, NULL);
+ sstack = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+ if (sstack == MAP_FAILED) {
+ perror("mmap()");
+ return EXIT_FAILURE;
+ }
+ stk.ss_sp = sstack;
+ stk.ss_size = SIGSTKSZ;
+ stk.ss_flags = SS_ONSTACK | SS_AUTODISARM;
+ err = sigaltstack(&stk, NULL);
+ if (err) {
+ perror("[FAIL]\tsigaltstack(SS_ONSTACK | SS_AUTODISARM)");
+ stk.ss_flags = SS_ONSTACK;
+ }
+ err = sigaltstack(&stk, NULL);
+ if (err) {
+ perror("[FAIL]\tsigaltstack(SS_ONSTACK)");
+ return EXIT_FAILURE;
+ }
+
+ ustack = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+ if (ustack == MAP_FAILED) {
+ perror("mmap()");
+ return EXIT_FAILURE;
+ }
+ getcontext(&uc);
+ uc.uc_link = NULL;
+ uc.uc_stack.ss_sp = ustack;
+ uc.uc_stack.ss_size = SIGSTKSZ;
+ makecontext(&uc, switch_fn, 0);
+ raise(SIGUSR1);
+
+ err = sigaltstack(NULL, &stk);
+ if (err) {
+ perror("[FAIL]\tsigaltstack()");
+ exit(EXIT_FAILURE);
+ }
+ if (stk.ss_flags != 0) {
+ printf("[FAIL]\tss_flags=%i, should be 0\n",
+ stk.ss_flags);
+ exit(EXIT_FAILURE);
+ }
+ printf("[OK]\tsigaltstack is enabled after signal\n");
+
+ printf("[OK]\tTest passed\n");
+ return 0;
+}
--
2.7.2
This patch implements the SS_AUTODISARM flag that can be ORed with
SS_ONSTACK when forming ss_flags.
When this flag is set, sigaltstack will be disabled when entering
the signal handler; more precisely, after saving sas to uc_stack.
When leaving the signal handler, the sigaltstack is restored by
uc_stack.
When this flag is used, it is safe to switch from sighandler with
swapcontext(). Without this flag, the subsequent signal will corrupt
the state of the switched-away sighandler.
To detect the support of this functionality, one can do
err = sigaltstack(SS_DISABLE | SS_AUTODISARM);
if (err && errno == EINVAL) unsupported();
Signed-off-by: Stas Sergeev <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Richard Weinberger <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Oleg Nesterov <[email protected]>
CC: Tejun Heo <[email protected]>
CC: Heinrich Schuchardt <[email protected]>
CC: Jason Low <[email protected]>
CC: Andrea Arcangeli <[email protected]>
CC: Frederic Weisbecker <[email protected]>
CC: Konstantin Khlebnikov <[email protected]>
CC: Josh Triplett <[email protected]>
CC: "Eric W. Biederman" <[email protected]>
CC: Aleksa Sarai <[email protected]>
CC: "Amanieu d'Antras" <[email protected]>
CC: Paul Moore <[email protected]>
CC: Sasha Levin <[email protected]>
CC: Palmer Dabbelt <[email protected]>
CC: Vladimir Davydov <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Andy Lutomirski <[email protected]>
---
include/linux/sched.h | 8 ++++++++
include/linux/signal.h | 4 +++-
include/uapi/linux/signal.h | 4 +++-
kernel/fork.c | 2 +-
kernel/signal.c | 10 ++++++++--
5 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a10494a..26201cd 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1587,6 +1587,7 @@ struct task_struct {
unsigned long sas_ss_sp;
size_t sas_ss_size;
+ unsigned sas_ss_flags;
struct callback_head *task_works;
@@ -2573,6 +2574,13 @@ static inline int sas_ss_flags(unsigned long sp)
return on_sig_stack(sp) ? SS_ONSTACK : 0;
}
+static inline void sas_ss_reset(struct task_struct *p)
+{
+ p->sas_ss_sp = 0;
+ p->sas_ss_size = 0;
+ p->sas_ss_flags = SS_DISABLE;
+}
+
static inline unsigned long sigsp(unsigned long sp, struct ksignal *ksig)
{
if (unlikely((ksig->ka.sa.sa_flags & SA_ONSTACK)) && ! sas_ss_flags(sp))
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 92557bb..3fbe814 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -432,8 +432,10 @@ int __save_altstack(stack_t __user *, unsigned long);
stack_t __user *__uss = uss; \
struct task_struct *t = current; \
put_user_ex((void __user *)t->sas_ss_sp, &__uss->ss_sp); \
- put_user_ex(sas_ss_flags(sp), &__uss->ss_flags); \
+ put_user_ex(t->sas_ss_flags, &__uss->ss_flags); \
put_user_ex(t->sas_ss_size, &__uss->ss_size); \
+ if (t->sas_ss_flags & SS_AUTODISARM) \
+ sas_ss_reset(t); \
} while (0);
#ifdef CONFIG_PROC_FS
diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index 7c73165..7388260 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -7,7 +7,9 @@
#define SS_ONSTACK 1
#define SS_DISABLE 2
+/* bit-flags */
+#define SS_AUTODISARM (1 << 4) /* disable sas during sighandling */
/* mask for all SS_xxx flags */
-#define SS_FLAG_BITS 0
+#define SS_FLAG_BITS SS_AUTODISARM
#endif /* _UAPI_LINUX_SIGNAL_H */
diff --git a/kernel/fork.c b/kernel/fork.c
index 2e391c7..68c8716 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1483,7 +1483,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
* sigaltstack should be cleared when sharing the same VM
*/
if ((clone_flags & (CLONE_VM|CLONE_VFORK)) == CLONE_VM)
- p->sas_ss_sp = p->sas_ss_size = 0;
+ sas_ss_reset(p);
/*
* Syscall tracing and stepping should be turned off in the
diff --git a/kernel/signal.c b/kernel/signal.c
index 9a24bc3..e1096ad 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3133,6 +3133,7 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
current->sas_ss_sp = (unsigned long) ss_sp;
current->sas_ss_size = ss_size;
+ current->sas_ss_flags = ss_flags;
}
error = 0;
@@ -3163,9 +3164,14 @@ int restore_altstack(const stack_t __user *uss)
int __save_altstack(stack_t __user *uss, unsigned long sp)
{
struct task_struct *t = current;
- return __put_user((void __user *)t->sas_ss_sp, &uss->ss_sp) |
- __put_user(sas_ss_flags(sp), &uss->ss_flags) |
+ int err = __put_user((void __user *)t->sas_ss_sp, &uss->ss_sp) |
+ __put_user(t->sas_ss_flags, &uss->ss_flags) |
__put_user(t->sas_ss_size, &uss->ss_size);
+ if (err)
+ return err;
+ if (t->sas_ss_flags & SS_AUTODISARM)
+ sas_ss_reset(t);
+ return 0;
}
#ifdef CONFIG_COMPAT
--
2.7.2
Currently x86's get_sigframe() checks for "current->sas_ss_size"
to determine whether there is a need to switch to sigaltstack.
The common practice used by all other arches is to check for
sas_ss_flags(sp) == 0
This patch makes the code consistent with other arches.
The slight complexity of the patch is added by the optimization on
!sigstack check that was requested by Andy Lutomirski: sas_ss_flags(sp)==0
already implies that we are not on a sigstack, so the code is shuffled
to avoid the duplicate checking.
This patch have no any user-visible impact. It is purely a cleanup.
CC: [email protected]
CC: Andy Lutomirski <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: "H. Peter Anvin" <[email protected]>
CC: [email protected]
CC: Borislav Petkov <[email protected]>
CC: Brian Gerst <[email protected]>
CC: Oleg Nesterov <[email protected]>
CC: Richard Weinberger <[email protected]>
Signed-off-by: Stas Sergeev <[email protected]>
---
arch/x86/kernel/signal.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index cb6282c..285183b 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -213,18 +213,17 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
if (config_enabled(CONFIG_X86_64))
sp -= 128;
- if (!onsigstack) {
- /* This is the X/Open sanctioned signal stack switching. */
- if (ka->sa.sa_flags & SA_ONSTACK) {
- if (current->sas_ss_size)
- sp = current->sas_ss_sp + current->sas_ss_size;
- } else if (config_enabled(CONFIG_X86_32) &&
- (regs->ss & 0xffff) != __USER_DS &&
- !(ka->sa.sa_flags & SA_RESTORER) &&
- ka->sa.sa_restorer) {
- /* This is the legacy signal stack switching. */
- sp = (unsigned long) ka->sa.sa_restorer;
- }
+ /* This is the X/Open sanctioned signal stack switching. */
+ if (ka->sa.sa_flags & SA_ONSTACK) {
+ if (sas_ss_flags(sp) == 0)
+ sp = current->sas_ss_sp + current->sas_ss_size;
+ } else if (config_enabled(CONFIG_X86_32) &&
+ !onsigstack &&
+ (regs->ss & 0xffff) != __USER_DS &&
+ !(ka->sa.sa_flags & SA_RESTORER) &&
+ ka->sa.sa_restorer) {
+ /* This is the legacy signal stack switching. */
+ sp = (unsigned long) ka->sa.sa_restorer;
}
if (fpu->fpstate_active) {
--
2.7.2
This patch adds SS_FLAG_BITS - the mask that splits sigaltstack
mode values and bit-flags. Since there is no bit-flags yet, the
mask is defined to 0. The flags are added by subsequent patches.
With every new flag, the mask should have the appropriate bit cleared.
This makes sure if some flag is tried on a kernel that doesn't
support it, the EINVAL error will be returned, because such a
flag will be treated as an invalid mode rather than the bit-flag.
That way the existence of the particular features can be probed
at run-time.
This change was suggested by Andy Lutomirski:
https://lkml.org/lkml/2016/3/6/158
Signed-off-by: Stas Sergeev <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Oleg Nesterov <[email protected]>
CC: "Peter Zijlstra (Intel)" <[email protected]>
CC: "Amanieu d'Antras" <[email protected]>
CC: Michal Hocko <[email protected]>
CC: Richard Weinberger <[email protected]>
CC: Vladimir Davydov <[email protected]>
CC: Sasha Levin <[email protected]>
CC: [email protected]
---
include/uapi/linux/signal.h | 3 +++
kernel/signal.c | 16 ++++++----------
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/include/uapi/linux/signal.h b/include/uapi/linux/signal.h
index e1bd50c2..7c73165 100644
--- a/include/uapi/linux/signal.h
+++ b/include/uapi/linux/signal.h
@@ -7,4 +7,7 @@
#define SS_ONSTACK 1
#define SS_DISABLE 2
+/* mask for all SS_xxx flags */
+#define SS_FLAG_BITS 0
+
#endif /* _UAPI_LINUX_SIGNAL_H */
diff --git a/kernel/signal.c b/kernel/signal.c
index 0508544..9a24bc3 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3100,7 +3100,8 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
if (uss) {
void __user *ss_sp;
size_t ss_size;
- int ss_flags;
+ unsigned ss_flags;
+ int ss_mode;
error = -EFAULT;
if (!access_ok(VERIFY_READ, uss, sizeof(*uss)))
@@ -3115,18 +3116,13 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
if (on_sig_stack(sp))
goto out;
+ ss_mode = ss_flags & ~SS_FLAG_BITS;
error = -EINVAL;
- /*
- * Note - this code used to test ss_flags incorrectly:
- * old code may have been written using ss_flags==0
- * to mean ss_flags==SS_ONSTACK (as this was the only
- * way that worked) - this fix preserves that older
- * mechanism.
- */
- if (ss_flags != SS_DISABLE && ss_flags != SS_ONSTACK && ss_flags != 0)
+ if (ss_mode != SS_DISABLE && ss_mode != SS_ONSTACK &&
+ ss_mode != 0)
goto out;
- if (ss_flags == SS_DISABLE) {
+ if (ss_mode == SS_DISABLE) {
ss_size = 0;
ss_sp = NULL;
} else {
--
2.7.2
So what is the status of the below?
I haven't seen any NAKs or objections, yet it seems not
applied 3 weeks later. What should I do now?
07.03.2016 19:52, Stas Sergeev пишет:
> The following patches make it possible to use swapcontext()
> in a sighandler that works on sigaltstack.
> The approach is inspired by Andy Lutomirski's suggestion that
> sigaltstack should disarm itself after saving into uc_stack:
> https://lkml.org/lkml/2016/2/1/594
>
> I add the SS_AUTODISARM flag that does exactly that.
> On sighandler exit, the sigaltstack is restored from uc_stack.
> Another possible name could be SS_ONESHOT, but, since it gets
> always re-enabled, I choose SS_AUTODISARM.
>
> Change since v5:
> - Fix description of patch 4/4
>
> Change since v4:
> - Implement this Andy Lutomirski's suggestion:
> https://lkml.org/lkml/2016/3/6/158
> that allows the run-time probing of the existence of the
> flags added in the future.
>
> [PATCH 1/4] [Cleanup] x86: signal: unify the sigaltstack check with
> A clean-up patch that unifies x86's sigaltstack handling with
> other arches.
> [PATCH 2/4] sigaltstack: preparations for adding new SS_xxx flags
> Andy's suggested changes
> [PATCH 3/4] sigaltstack: implement SS_AUTODISARM flag
> This patch implements SS_AUTODISARM flag
> [PATCH 4/4] selftests: Add test for
> This patch adds the selftest code for new functionality
>
> CC: [email protected]
> CC: [email protected]
> CC: Andy Lutomirski <[email protected]>
> CC: Oleg Nesterov <[email protected]>
> CC: Shuah Khan <[email protected]>
> CC: Ingo Molnar <[email protected]>
>
> Diffstat:
> arch/x86/kernel/signal.c | 23 +--
> include/linux/sched.h | 8 +
> include/linux/signal.h | 4
> include/uapi/linux/signal.h | 7 +
> kernel/fork.c | 2
> kernel/signal.c | 26 ++--
> tools/testing/selftests/Makefile | 1
> tools/testing/selftests/sigaltstack/Makefile | 8 +
> tools/testing/selftests/sigaltstack/sas.c | 156 +++++++++++++++++++++++++++
> 9 files changed, 208 insertions(+), 27 deletions(-)
>
>