Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1393662 > unrolled thread
| Started by | Andy Lutomirski <luto@kernel.org> |
|---|---|
| First post | 2016-05-03 19:40 +0200 |
| Last post | 2016-05-04 08:30 +0200 |
| Articles | 12 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH 0/4] SS_AUTODISARM fixes and an ABI change Andy Lutomirski <luto@kernel.org> - 2016-05-03 19:40 +0200
[PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack Andy Lutomirski <luto@kernel.org> - 2016-05-03 19:40 +0200
Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack Ingo Molnar <mingo@kernel.org> - 2016-05-04 08:40 +0200
Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack Andy Lutomirski <luto@amacapital.net> - 2016-05-05 01:10 +0200
[tip:core/signals] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack() tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-05-04 09:20 +0200
Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack Stas Sergeev <stsp@list.ru> - 2016-05-07 16:40 +0200
Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack Andy Lutomirski <luto@amacapital.net> - 2016-05-09 03:40 +0200
Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack Stas Sergeev <stsp@list.ru> - 2016-05-09 04:10 +0200
[PATCH 4/4] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31) Andy Lutomirski <luto@kernel.org> - 2016-05-03 19:40 +0200
[tip:core/signals] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31) tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-05-04 09:20 +0200
Re: [PATCH 4/4] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31) Stas Sergeev <stsp@list.ru> - 2016-05-07 17:20 +0200
Re: [PATCH 0/4] SS_AUTODISARM fixes and an ABI change Ingo Molnar <mingo@kernel.org> - 2016-05-04 08:30 +0200
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2016-05-03 19:40 +0200 |
| Subject | [PATCH 0/4] SS_AUTODISARM fixes and an ABI change |
| Message-ID | <ruMsG-3Sd-9@gated-at.bofh.it> |
The first three are fixes IMO. The fourth changes the SS_AUTODISARM bit. I'm assuming that's okay, as the bit has existed in -tip for less than a day. Andy Lutomirski (4): signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack selftests/sigaltstack: Fix the sas test on old kernels signals/sigaltstack: Report current flag bits in sigaltstack() signals/sigaltstack: Change SS_AUTODISARM to (1U << 31) include/linux/sched.h | 12 +++++++++ include/uapi/linux/signal.h | 2 +- kernel/signal.c | 3 ++- tools/testing/selftests/sigaltstack/sas.c | 42 +++++++++++++++++++++++-------- 4 files changed, 46 insertions(+), 13 deletions(-) -- 2.5.5
[toc] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2016-05-03 19:40 +0200 |
| Subject | [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack |
| Message-ID | <ruMsI-3Sd-57@gated-at.bofh.it> |
| In reply to | #1393662 |
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
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2016-05-04 08:40 +0200 |
| Subject | Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack |
| Message-ID | <ruYDx-77h-39@gated-at.bofh.it> |
| In reply to | #1393663 |
* Andy Lutomirski <luto@kernel.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@list.ru> Presuably that SOB from Stas is stray, as there's no matching From: line? I've removed it. Thanks, Ingo
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-05-05 01:10 +0200 |
| Subject | Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack |
| Message-ID | <rve5A-4JL-9@gated-at.bofh.it> |
| In reply to | #1394022 |
On May 3, 2016 11:32 PM, "Ingo Molnar" <mingo@kernel.org> wrote: > > > * Andy Lutomirski <luto@kernel.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@list.ru> > > 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
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Andy Lutomirski <tipbot@zytor.com> |
|---|---|
| Date | 2016-05-04 09:20 +0200 |
| Subject | [tip:core/signals] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack() |
| Message-ID | <ruZgf-7OK-45@gated-at.bofh.it> |
| In reply to | #1393663 |
Commit-ID: c876eeab6432687846d4cd5fe1e43dbc348de134
Gitweb: http://git.kernel.org/tip/c876eeab6432687846d4cd5fe1e43dbc348de134
Author: Andy Lutomirski <luto@kernel.org>
AuthorDate: Tue, 3 May 2016 10:31:49 -0700
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 4 May 2016 08:34:13 +0200
signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack()
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: Andy Lutomirski <luto@kernel.org>
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: Stas Sergeev <stsp@list.ru>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vladimir Davydov <vdavydov@parallels.com>
Cc: linux-api@vger.kernel.org
Link: http://lkml.kernel.org/r/c46bee4654ca9e68c498462fd11746e2bd0d98c8.1462296606.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@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 2950c5c..77fd49f 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 SS_AUTODISARM then, by construction, we
+ * can't be on the signal stack unless user code deliberately set
+ * SS_AUTODISARM when we were already on it.
+ *
+ * This improves 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;
[toc] | [prev] | [next] | [standalone]
| From | Stas Sergeev <stsp@list.ru> |
|---|---|
| Date | 2016-05-07 16:40 +0200 |
| Subject | Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack |
| Message-ID | <rwbyG-398-5@gated-at.bofh.it> |
| In reply to | #1393663 |
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?
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?
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2016-05-09 03:40 +0200 |
| Subject | Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack |
| Message-ID | <rwIkV-1K9-1@gated-at.bofh.it> |
| In reply to | #1396312 |
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?
[toc] | [prev] | [next] | [standalone]
| From | Stas Sergeev <stsp@list.ru> |
|---|---|
| Date | 2016-05-09 04:10 +0200 |
| Subject | Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack |
| Message-ID | <rwINY-2jj-5@gated-at.bofh.it> |
| In reply to | #1396531 |
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.
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2016-05-03 19:40 +0200 |
| Subject | [PATCH 4/4] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31) |
| Message-ID | <ruMsI-3Sd-61@gated-at.bofh.it> |
| In reply to | #1393662 |
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
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Andy Lutomirski <tipbot@zytor.com> |
|---|---|
| Date | 2016-05-04 09:20 +0200 |
| Subject | [tip:core/signals] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31) |
| Message-ID | <ruZgd-7OK-1@gated-at.bofh.it> |
| In reply to | #1393668 |
Commit-ID: 91c6180572e2fec71701d646ffc40ad30986275c Gitweb: http://git.kernel.org/tip/91c6180572e2fec71701d646ffc40ad30986275c Author: Andy Lutomirski <luto@kernel.org> AuthorDate: Tue, 3 May 2016 10:31:52 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Wed, 4 May 2016 08:34:14 +0200 signals/sigaltstack: Change SS_AUTODISARM to (1U << 31) 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. Signed-off-by: Andy Lutomirski <luto@kernel.org> 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: Stas Sergeev <stsp@list.ru> Cc: Tejun Heo <tj@kernel.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Vladimir Davydov <vdavydov@parallels.com> Cc: linux-api@vger.kernel.org Link: http://lkml.kernel.org/r/bb996508a600af14b406810c3d58fe0e0d0afe0d.1462296606.git.luto@kernel.org Signed-off-by: Ingo Molnar <mingo@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 7388260..cd0804b 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 4280d06..1bb0125 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;
[toc] | [prev] | [next] | [standalone]
| From | Stas Sergeev <stsp@list.ru> |
|---|---|
| Date | 2016-05-07 17:20 +0200 |
| Subject | Re: [PATCH 4/4] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31) |
| Message-ID | <rwcbo-3Lb-5@gated-at.bofh.it> |
| In reply to | #1393668 |
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@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 */ 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.
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2016-05-04 08:30 +0200 |
| Message-ID | <ruYtQ-71w-13@gated-at.bofh.it> |
| In reply to | #1393662 |
* Andy Lutomirski <luto@kernel.org> wrote: > The first three are fixes IMO. The fourth changes the SS_AUTODISARM > bit. I'm assuming that's okay, as the bit has existed in -tip for > less than a day. Yeah, it's absolutely OK - I made it a standalone tree so we could even rebase it, but I think authorship and intent is clearer with your series on top of Stas's original commits. Thanks, Ingo
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web