linux-kernel - [PATCH 2/2] sigaltstack: remove EPERM check to make swapcontext() usable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56905FF0.4040406@list.ru>
Date:	Sat, 9 Jan 2016 04:18:40 +0300
From:	Stas Sergeev <stsp@...t.ru>
To:	Linux kernel <linux-kernel@...r.kernel.org>
Cc:	linux-api@...r.kernel.org, Andy Lutomirski <luto@...capital.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Amanieu d'Antras <amanieu@...il.com>,
	Richard Weinberger <richard@....at>,
	Palmer Dabbelt <palmer@...belt.com>,
	Vladimir Davydov <vdavydov@...allels.com>
Subject: [PATCH 2/2] sigaltstack: remove EPERM check to make swapcontext()
 usable


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 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. The oss->ss_flags
will then return SS_DISABLE, which doesn't seem to contradict the
(very ambiguous) man page wording, namely:
        SS_ONSTACK
               The process is currently executing on the alternate signal
               stack. (Note that it is not possible to change the alternate
               signal stack if the process is currently executing on it.)

        SS_DISABLE
               The alternate signal stack is currently disabled.

It seems both the above cases apply when executing on sigaltstack with
sigaltstack being currently disabled, so hope no one really cares.

CC: Andrew Morton <akpm@...ux-foundation.org>
CC: Oleg Nesterov <oleg@...hat.com>
CC: "Amanieu d'Antras" <amanieu@...il.com>
CC: Richard Weinberger <richard@....at>
CC: Andy Lutomirski <luto@...capital.net>
CC: Palmer Dabbelt <palmer@...belt.com>
CC: Vladimir Davydov <vdavydov@...allels.com>
CC: linux-kernel@...r.kernel.org
CC: linux-api@...r.kernel.org

Signed-off-by: Stas Sergeev <stsp@...rs.sourceforge.net>
---
  kernel/signal.c | 15 +++++----------
  1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index f3f1f7a..0a6af54 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3111,18 +3111,13 @@ 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.
+         * Note - this code used to test on_sig_stack(sp) and
+         * return -EPERM. But we need at least SS_DISABLE to
+         * work while on sigaltstack, so the check was removed.
           */
+
+        error = -EINVAL;
          if (ss_flags != SS_DISABLE && ss_flags != SS_ONSTACK && 
ss_flags != 0)
              goto out;

-- 
2.4.3