x86: signal: don't do sas_ss_reset() until we are certain that sigframe won't be...

Currently we handle SS_AUTODISARM as soon as we have stored the altstack
settings into sigframe - that's the point when we have set the things up
for eventual sigreturn to restore the old settings.  And if we manage to
set the sigframe up (we are not done with that yet), everything's fine.
However, in case of failure we end up with sigframe-to-be abandoned and
SIGSEGV force-delivered.  And in that case we end up with inconsistent
rules - late failures have altstack reset, early ones do not.

It's trivial to get consistent behaviour - just handle SS_AUTODISARM once
we have set the sigframe up and are committed to entering the handler,
i.e.  in signal_delivered().

Link: https://lore.kernel.org/lkml/20200404170604.GN23230@ZenIV.linux.org.uk/
Link: https://github.com/ClangBuiltLinux/linux/issues/876
Link: https://lkml.kernel.org/r/20210422230846.1756380-1-ndesaulniers@google.com
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/include/linux/compat.h b/include/linux/compat.h
index 8855b1b702b2a7e84ec0d8c75491f4347040d0d1..c270124e44020c4ebc2c36c6d03f010bc553f1ba 100644 (file)
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -532,8 +532,6 @@ int __compat_save_altstack(compat_stack_t __user *, unsigned long);
                        &__uss->ss_sp, label); \
        unsafe_put_user(t->sas_ss_flags, &__uss->ss_flags, label); \
        unsafe_put_user(t->sas_ss_size, &__uss->ss_size, label); \
-       if (t->sas_ss_flags & SS_AUTODISARM) \
-               sas_ss_reset(t); \
 } while (0);
 
 /*

diff --git a/include/linux/signal.h b/include/linux/signal.h
index 5160fd45e5cab878b2393b10ee7336f8a1a9fea9..3454c7ff077870a9a6187cdb88d6818c751daa28 100644 (file)
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -462,8 +462,6 @@ int __save_altstack(stack_t __user *, unsigned long);
        unsafe_put_user((void __user *)t->sas_ss_sp, &__uss->ss_sp, label); \
        unsafe_put_user(t->sas_ss_flags, &__uss->ss_flags, label); \
        unsafe_put_user(t->sas_ss_size, &__uss->ss_size, label); \
-       if (t->sas_ss_flags & SS_AUTODISARM) \
-               sas_ss_reset(t); \
 } while (0);
 
 #ifdef CONFIG_PROC_FS

diff --git a/kernel/signal.c b/kernel/signal.c
index 30a0bee5ff9bb88e8f18c49db9c06eaff7b8d59a..6b4df88cc630b798f077778fdf81dd83634e82a4 100644 (file)
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2829,6 +2829,8 @@ static void signal_delivered(struct ksignal *ksig, int stepping)
        if (!(ksig->ka.sa.sa_flags & SA_NODEFER))
                sigaddset(&blocked, ksig->sig);
        set_current_blocked(&blocked);
+       if (current->sas_ss_flags & SS_AUTODISARM)
+               sas_ss_reset(current);
        tracehook_signal_handler(stepping);
 }
 
@@ -4147,11 +4149,7 @@ int __save_altstack(stack_t __user *uss, unsigned long sp)
        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;
+       return err;
 }
 
 #ifdef CONFIG_COMPAT
@@ -4206,11 +4204,7 @@ int __compat_save_altstack(compat_stack_t __user *uss, unsigned long 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;
+       return err;
 }
 #endif