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

[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

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

[Ingo Molnar]

* Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-cmBhpYW9OiY@public.gmane.org>

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

Thanks,

	Ingo

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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>
> * Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-cmBhpYW9OiY@public.gmane.org>
>
> 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-cmBhpYW9OiY@public.gmane.org>
> Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
> Cc: Aleksa Sarai <cyphar-gVpy/LI/lHzQT0dZR+AlfA@public.gmane.org>
> Cc: Amanieu d'Antras <amanieu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Andrea Arcangeli <aarcange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
> Cc: Brian Gerst <brgerst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Denys Vlasenko <dvlasenk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
> Cc: Frederic Weisbecker <fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> Cc: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
> Cc: Jason Low <jason.low2-VXdhtT5mjnY@public.gmane.org>
> Cc: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
> Cc: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Palmer Dabbelt <palmer-96lFi9zoCfxBDgjK7y7TUQ@public.gmane.org>
> Cc: Paul Moore <pmoore-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> Cc: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> Cc: Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org>
> Cc: Sasha Levin <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> Cc: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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 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-cmBhpYW9OiY@public.gmane.org> 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-cmBhpYW9OiY@public.gmane.org>
>>> Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
>>> Cc: Aleksa Sarai <cyphar-gVpy/LI/lHzQT0dZR+AlfA@public.gmane.org>
>>> Cc: Amanieu d'Antras <amanieu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: Andrea Arcangeli <aarcange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>>> Cc: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
>>> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
>>> Cc: Brian Gerst <brgerst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: Denys Vlasenko <dvlasenk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> Cc: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
>>> Cc: Frederic Weisbecker <fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
>>> Cc: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
>>> Cc: Jason Low <jason.low2-VXdhtT5mjnY@public.gmane.org>
>>> Cc: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
>>> Cc: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
>>> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>>> Cc: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> Cc: Palmer Dabbelt <palmer-96lFi9zoCfxBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Paul Moore <pmoore-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>> Cc: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
>>> Cc: Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org>
>>> Cc: Sasha Levin <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>>> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>>> Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>>> Cc: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-cmBhpYW9OiY@public.gmane.org> wrote:
>
> 09.05.2016 04:32, Andy Lutomirski пишет:
>
>> On May 7, 2016 7:38 AM, "Stas Sergeev" <stsp-cmBhpYW9OiY@public.gmane.org> 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-cmBhpYW9OiY@public.gmane.org>
>>>> Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
>>>> Cc: Aleksa Sarai <cyphar-gVpy/LI/lHzQT0dZR+AlfA@public.gmane.org>
>>>> Cc: Amanieu d'Antras <amanieu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Cc: Andrea Arcangeli <aarcange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>>>> Cc: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
>>>> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
>>>> Cc: Brian Gerst <brgerst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Cc: Denys Vlasenko <dvlasenk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> Cc: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
>>>> Cc: Frederic Weisbecker <fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Cc: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
>>>> Cc: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
>>>> Cc: Jason Low <jason.low2-VXdhtT5mjnY@public.gmane.org>
>>>> Cc: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
>>>> Cc: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
>>>> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>>>> Cc: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> Cc: Palmer Dabbelt <palmer-96lFi9zoCfxBDgjK7y7TUQ@public.gmane.org>
>>>> Cc: Paul Moore <pmoore-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>>> Cc: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
>>>> Cc: Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org>
>>>> Cc: Sasha Levin <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>>>> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>>>> Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>>>> Cc: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>>> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-cmBhpYW9OiY@public.gmane.org> wrote:
>> 09.05.2016 04:32, Andy Lutomirski пишет:
>>
>>> On May 7, 2016 7:38 AM, "Stas Sergeev" <stsp-cmBhpYW9OiY@public.gmane.org> 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-cmBhpYW9OiY@public.gmane.org>
>>>>> Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
>>>>> Cc: Aleksa Sarai <cyphar-gVpy/LI/lHzQT0dZR+AlfA@public.gmane.org>
>>>>> Cc: Amanieu d'Antras <amanieu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> Cc: Andrea Arcangeli <aarcange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>>>>> Cc: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
>>>>> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
>>>>> Cc: Brian Gerst <brgerst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> Cc: Denys Vlasenko <dvlasenk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>> Cc: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
>>>>> Cc: Frederic Weisbecker <fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> Cc: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
>>>>> Cc: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
>>>>> Cc: Jason Low <jason.low2-VXdhtT5mjnY@public.gmane.org>
>>>>> Cc: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
>>>>> Cc: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
>>>>> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>>>>> Cc: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>> Cc: Palmer Dabbelt <palmer-96lFi9zoCfxBDgjK7y7TUQ@public.gmane.org>
>>>>> Cc: Paul Moore <pmoore-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>> Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>>>> Cc: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
>>>>> Cc: Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org>
>>>>> Cc: Sasha Levin <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>>>>> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>>>>> Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>>>>> Cc: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>>>> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>> Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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.

[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

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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Stas Sergeev <stsp-cmBhpYW9OiY@public.gmane.org>
> Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
> Cc: Brian Gerst <brgerst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Denys Vlasenko <dvlasenk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> Cc: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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 2/4] selftests/sigaltstack: Fix the sas test on old kernels

[Andy Lutomirski]

On May 7, 2016 8:02 AM, "Stas Sergeev" <stsp-cmBhpYW9OiY@public.gmane.org> wrote:
>
> 03.05.2016 20:31, Andy Lutomirski пишет:
>
>> The handling for old kernels was wrong.  Fix it.
>>
>> Reported-by: Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Stas Sergeev <stsp-cmBhpYW9OiY@public.gmane.org>
>> Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
>> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>> Cc: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
>> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
>> Cc: Brian Gerst <brgerst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Denys Vlasenko <dvlasenk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Cc: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
>> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
>> Cc: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>> Cc: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
>> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
>> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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.

[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

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

[Ingo Molnar]

* Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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-cmBhpYW9OiY@public.gmane.org>

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

Thanks,

	Ingo

[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 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-cmBhpYW9OiY@public.gmane.org>
> Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
> Cc: Aleksa Sarai <cyphar-gVpy/LI/lHzQT0dZR+AlfA@public.gmane.org>
> Cc: Amanieu d'Antras <amanieu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Andrea Arcangeli <aarcange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
> Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
> Cc: Brian Gerst <brgerst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Denys Vlasenko <dvlasenk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
> Cc: Frederic Weisbecker <fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: H. Peter Anvin <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
> Cc: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
> Cc: Jason Low <jason.low2-VXdhtT5mjnY@public.gmane.org>
> Cc: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
> Cc: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Palmer Dabbelt <palmer-96lFi9zoCfxBDgjK7y7TUQ@public.gmane.org>
> Cc: Paul Moore <pmoore-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> Cc: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> Cc: Richard Weinberger <richard-/L3Ra7n9ekc@public.gmane.org>
> Cc: Sasha Levin <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Cc: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> Cc: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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.