[PATCH 4/4] sigaltstack: allow disabling and re-enabling sas within sighandler

[Stas Sergeev]

linux implements the sigaltstack() in a way that makes it impossible to
use with swapcontext(). Per the man page, sigaltstack is allowed to return
EPERM if the process is altering its sigaltstack while running on
sigaltstack.
This is likely needed to consistently return oss->ss_flags, that indicates
whether the process is being on sigaltstack or not.
Unfortunately, linux takes that permission to return EPERM too literally:
it returns EPERM even if you don't want to change to another sigaltstack,
but only want to temporarily disable sigaltstack with SS_DISABLE.
You can't use swapcontext() without disabling sigaltstack first, or the
stack will be re-used and overwritten by a subsequent signal.

With this patch, disabling sigaltstack inside a signal handler became
possible, and the swapcontext() can then be used safely. After switching
back to the sighandler, the app can re-enable the sigatlstack.
The oss->ss_flags will correctly indicate the current use of sigaltstack,
even if it is temporarily disabled. Any attempt to modify the sigaltstack
(rather than to disable or re-enable it) within the sighandler, will still
be punished with EPERM as suggested by POSIX.

CC: Ingo Molnar <mingo@xxxxxxxxxx>
CC: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
CC: Oleg Nesterov <oleg@xxxxxxxxxx>
CC: "Amanieu d'Antras" <amanieu@xxxxxxxxx>
CC: Richard Weinberger <richard@xxxxxx>
CC: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
CC: Tejun Heo <tj@xxxxxxxxxx>
CC: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
CC: Jason Low <jason.low2@xxxxxx>
CC: Heinrich Schuchardt <xypron.glpk@xxxxxx>
CC: Andrea Arcangeli <aarcange@xxxxxxxxxx>
CC: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx>
CC: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
CC: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
CC: Aleksa Sarai <cyphar@xxxxxxxxxx>
CC: Paul Moore <pmoore@xxxxxxxxxx>
CC: Palmer Dabbelt <palmer@xxxxxxxxxxx>
CC: Vladimir Davydov <vdavydov@xxxxxxxxxxxxx>
CC: linux-kernel@xxxxxxxxxxxxxxx
CC: linux-api@xxxxxxxxxxxxxxx

Signed-off-by: Stas Sergeev <stsp@xxxxxxxxxxxxxxxxxxxxx>
---
 include/linux/sched.h  |  8 ++++++--
 include/linux/signal.h |  2 +-
 kernel/fork.c          |  4 +++-
 kernel/signal.c        | 54 
+++++++++++++++++++++++++++++++++-----------------
 4 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a10494a..38f67dc 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;
+    int sas_ss_flags;

     struct callback_head *task_works;

@@ -2569,8 +2570,11 @@ static inline int sas_ss_flags(unsigned long sp)
 {
     if (!current->sas_ss_size)
         return SS_DISABLE;
-
-    return on_sig_stack(sp) ? SS_ONSTACK : 0;
+    if (on_sig_stack(sp))
+        return SS_ONSTACK;
+    if (current->sas_ss_flags == SS_DISABLE)
+        return SS_DISABLE;
+    return 0;
 }

 static inline unsigned long sigsp(unsigned long sp, struct ksignal *ksig)
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 92557bb..844b113 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -432,7 +432,7 @@ 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); \
 } while (0);

diff --git a/kernel/fork.c b/kernel/fork.c
index 2e391c7..ce840a9 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1482,8 +1482,10 @@ 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)
+    if ((clone_flags & (CLONE_VM|CLONE_VFORK)) == CLONE_VM) {
         p->sas_ss_sp = p->sas_ss_size = 0;
+        p->sas_ss_flags = SS_DISABLE;
+    }

     /*
      * Syscall tracing and stepping should be turned off in the
diff --git a/kernel/signal.c b/kernel/signal.c
index f3f1f7a..d19180e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3101,6 +3101,7 @@ do_sigaltstack (const stack_t __user *uss, stack_t 
__user *uoss, unsigned long s
         void __user *ss_sp;
         size_t ss_size;
         int ss_flags;
+        int onsigstack;

         error = -EFAULT;
         if (!access_ok(VERIFY_READ, uss, sizeof(*uss)))
@@ -3111,32 +3112,49 @@ do_sigaltstack (const stack_t __user *uss, 
stack_t __user *uoss, unsigned long s
         if (error)
             goto out;

-        error = -EPERM;
-        if (on_sig_stack(sp))
-            goto out;
-
         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;

-        if (ss_flags == SS_DISABLE) {
-            ss_size = 0;
-            ss_sp = NULL;
-        } else {
+        onsigstack = on_sig_stack(sp);
+        if (ss_size == 0) {
+            switch (ss_flags) {
+            case 0:
+                error = -EPERM;
+                if (onsigstack)
+                    goto out;
+                current->sas_ss_sp = 0;
+                current->sas_ss_size = 0;
+                current->sas_ss_flags = SS_DISABLE;
+                break;
+            case SS_ONSTACK:
+                /* re-enable previously disabled sas */
+                error = -EINVAL;
+                if (current->sas_ss_size == 0)
+                    goto out;
+                break;
+            default:
+                break;
+            }
+        } else if (ss_flags != SS_DISABLE) {
+            error = -EPERM;
+            if (onsigstack)
+                goto out;
             error = -ENOMEM;
             if (ss_size < MINSIGSTKSZ)
                 goto out;
+            current->sas_ss_sp = (unsigned long) ss_sp;
+            current->sas_ss_size = ss_size;
+            /* unfortunately POSIX forces us to treat 0
+             * as SS_ONSTACK here, and some legacy apps
+             * perhaps used that...
+             */
+            if (ss_flags == 0)
+                ss_flags = SS_ONSTACK;
         }

-        current->sas_ss_sp = (unsigned long) ss_sp;
-        current->sas_ss_size = ss_size;
+        if (ss_flags != 0)
+            current->sas_ss_flags = ss_flags;
     }

     error = 0;
@@ -3168,7 +3186,7 @@ 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) |
+        __put_user(t->sas_ss_flags, &uss->ss_flags) |
         __put_user(t->sas_ss_size, &uss->ss_size);
 }

--
2.5.0