SS_AUTODISARM fixes and an ABI change
cover SS_AUTODISARM fixes and an ABI change
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(-)
Signature · 1 line
2.5.5
PATCH 1/4 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.
--- include/linux/sched.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
include/linux/sched.h +12 -0
@@ -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;
Signature · 1 line
2.5.5
Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack
Quoted 11 lines · 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 ifSS_AUTODISARMis 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 <[email protected]>
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
Quoted 10 lines · Ingo Molnar
Quoted 6 lines · 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 ifSS_AUTODISARMis set.
Quoted 8 lines
Quoted 5 lines
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 <[email protected]>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.
Quoted 4 lines
Thanks,
Ingo
tip-bot for Andy Lutomirski
merge bot[tip:core/signals] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack()
In reply to Andy Lutomirskisignals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack()
Commit-ID: c876eeab6432687846d4cd5fe1e43dbc348de134 Gitweb: http://git.kernel.org/tip/c876eeab6432687846d4cd5fe1e43dbc348de134 Author: Andy Lutomirski <[email protected]> AuthorDate: Tue, 3 May 2016 10:31:49 -0700 Committer: Ingo Molnar <[email protected]> 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.
--- include/linux/sched.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
include/linux/sched.h +12 -0
@@ -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;
Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack
In reply to Andy Lutomirski03.05.2016 20:31, Andy Lutomirski пишет:
Quoted 51 lines
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 ifSS_AUTODISARMis 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 <[email protected]>Cc Al Viro <viro-RmSDqhL/[email protected]>Cc Aleksa Sarai <cyphar-gVpy/LI/[email protected]>Cc Amanieu d'Antras <[email protected]>Cc Andrea Arcangeli <[email protected]>Cc Andrew Morton <akpm-de/[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Denys Vlasenko <[email protected]>Cc Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>Cc Frederic Weisbecker <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Heinrich Schuchardt <[email protected]>Cc Jason Low <[email protected]>Cc Josh Triplett <[email protected]>Cc Konstantin Khlebnikov <[email protected]>Cc Linus Torvalds <torvalds-de/[email protected]>Cc Oleg Nesterov <[email protected]>Cc Palmer Dabbelt <[email protected]>Cc Paul Moore <[email protected]>Cc Pavel Emelyanov <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Richard Weinberger <richard-/[email protected]>Cc Sasha Levin <[email protected]>Cc Shuah Khan <[email protected]>Cc Tejun Heo <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Vladimir Davydov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- include/linux/sched.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)include/linux/sched.h +0
-0
include/linux/sched.h static inline int kill_cad_pid(int sig, int priv) · line 2582
Quoted 7 lines
include/linux/sched.h +4
-0@@ -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 setSS_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
Quoted 64 lines · Stas Sergeev
03.05.2016 20:31, Andy Lutomirski пишет:
Quoted 51 lines
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 ifSS_AUTODISARMis 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 <[email protected]>Cc Al Viro <[email protected]>Cc Aleksa Sarai <[email protected]>Cc Amanieu d'Antras <[email protected]>Cc Andrea Arcangeli <[email protected]>Cc Andrew Morton <[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Denys Vlasenko <[email protected]>Cc Eric W. Biederman <[email protected]>Cc Frederic Weisbecker <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Heinrich Schuchardt <[email protected]>Cc Jason Low <[email protected]>Cc Josh Triplett <[email protected]>Cc Konstantin Khlebnikov <[email protected]>Cc Linus Torvalds <[email protected]>Cc Oleg Nesterov <[email protected]>Cc Palmer Dabbelt <[email protected]>Cc Paul Moore <[email protected]>Cc Pavel Emelyanov <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Richard Weinberger <[email protected]>Cc Sasha Levin <[email protected]>Cc Shuah Khan <[email protected]>Cc Tejun Heo <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Vladimir Davydov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- include/linux/sched.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)include/linux/sched.h +0
-0include/linux/sched.h static inline int kill_cad_pid(int sig, int priv) · line 2582
Quoted 7 lines
include/linux/sched.h +4
-0@@ -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".
Quoted 6 lines
Anyway, I am a bit puzzled with this patch.
You say "unless user code deliberately set
SS_AUTODISARMwhen we were already on the it"
so what happens in case it actually does?
Stack corruption. Don't do that.
Quoted 7 lines
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
09.05.2016 04:32, Andy Lutomirski пишет:
Quoted 63 lines · Stas Sergeev
Quoted 62 lines
03.05.2016 20:31, Andy Lutomirski пишет:
Quoted 51 lines
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 ifSS_AUTODISARMis 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 <[email protected]>Cc Al Viro <viro-RmSDqhL/[email protected]>Cc Aleksa Sarai <cyphar-gVpy/LI/[email protected]>Cc Amanieu d'Antras <[email protected]>Cc Andrea Arcangeli <[email protected]>Cc Andrew Morton <akpm-de/[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Denys Vlasenko <[email protected]>Cc Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>Cc Frederic Weisbecker <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Heinrich Schuchardt <[email protected]>Cc Jason Low <[email protected]>Cc Josh Triplett <[email protected]>Cc Konstantin Khlebnikov <[email protected]>Cc Linus Torvalds <torvalds-de/[email protected]>Cc Oleg Nesterov <[email protected]>Cc Palmer Dabbelt <[email protected]>Cc Paul Moore <[email protected]>Cc Pavel Emelyanov <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Richard Weinberger <richard-/[email protected]>Cc Sasha Levin <[email protected]>Cc Shuah Khan <[email protected]>Cc Tejun Heo <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Vladimir Davydov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- include/linux/sched.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)include/linux/sched.h +0
-0include/linux/sched.h static inline int kill_cad_pid(int sig, int priv) · line 2582
Quoted 7 lines
include/linux/sched.h +4
-0@@ -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".
Quoted 7 lines
Quoted 6 lines
Anyway, I am a bit puzzled with this patch.
You say "unless user code deliberately set
SS_AUTODISARMwhen 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. :)
Quoted 14 lines
Quoted 7 lines
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 setSS_AUTODISARMand 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.
Quoted 4 lines
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.
Quoted 1 line
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
Quoted 103 lines · Stas Sergeev
09.05.2016 04:32, Andy Lutomirski пишет:
Quoted 65 lines · Stas Sergeev
Quoted 64 lines
03.05.2016 20:31, Andy Lutomirski пишет:
Quoted 51 lines
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 ifSS_AUTODISARMis 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 <[email protected]>Cc Al Viro <viro-RmSDqhL/[email protected]>Cc Aleksa Sarai <cyphar-gVpy/LI/[email protected]>Cc Amanieu d'Antras <[email protected]>Cc Andrea Arcangeli <[email protected]>Cc Andrew Morton <akpm-de/[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Denys Vlasenko <[email protected]>Cc Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>Cc Frederic Weisbecker <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Heinrich Schuchardt <[email protected]>Cc Jason Low <[email protected]>Cc Josh Triplett <[email protected]>Cc Konstantin Khlebnikov <[email protected]>Cc Linus Torvalds <torvalds-de/[email protected]>Cc Oleg Nesterov <[email protected]>Cc Palmer Dabbelt <[email protected]>Cc Paul Moore <[email protected]>Cc Pavel Emelyanov <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Richard Weinberger <richard-/[email protected]>Cc Sasha Levin <[email protected]>Cc Shuah Khan <[email protected]>Cc Tejun Heo <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Vladimir Davydov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- include/linux/sched.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)include/linux/sched.h +0
-0include/linux/sched.h static inline int kill_cad_pid(int sig, int priv) · line 2582
Quoted 7 lines
include/linux/sched.h +4
-0@@ -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".
Quoted 7 lines
Quoted 6 lines
Anyway, I am a bit puzzled with this patch.
You say "unless user code deliberately set
SS_AUTODISARMwhen 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. :)
Quoted 15 lines
Quoted 7 lines
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 setSS_AUTODISARMand 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.Quoted 4 lines
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.
Quoted 7 lines
Quoted 1 line
If this behavior would be problematic for you, can you explain why?
Only theoretically: if someone sets
SS_AUTODISARMinside 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.
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] signals/sigaltstack: If SS_AUTODISARM, bypass on_sig_stack
14.05.2016 07:18, Andy Lutomirski пишет:
Quoted 99 lines · Stas Sergeev
Quoted 98 lines
09.05.2016 04:32, Andy Lutomirski пишет:
Quoted 63 lines · Stas Sergeev
Quoted 62 lines
03.05.2016 20:31, Andy Lutomirski пишет:
Quoted 51 lines
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 ifSS_AUTODISARMis 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 <[email protected]>Cc Al Viro <viro-RmSDqhL/[email protected]>Cc Aleksa Sarai <cyphar-gVpy/LI/[email protected]>Cc Amanieu d'Antras <[email protected]>Cc Andrea Arcangeli <[email protected]>Cc Andrew Morton <akpm-de/[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Denys Vlasenko <[email protected]>Cc Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>Cc Frederic Weisbecker <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Heinrich Schuchardt <[email protected]>Cc Jason Low <[email protected]>Cc Josh Triplett <[email protected]>Cc Konstantin Khlebnikov <[email protected]>Cc Linus Torvalds <torvalds-de/[email protected]>Cc Oleg Nesterov <[email protected]>Cc Palmer Dabbelt <[email protected]>Cc Paul Moore <[email protected]>Cc Pavel Emelyanov <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Richard Weinberger <richard-/[email protected]>Cc Sasha Levin <[email protected]>Cc Shuah Khan <[email protected]>Cc Tejun Heo <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Vladimir Davydov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- include/linux/sched.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)include/linux/sched.h +0
-0include/linux/sched.h static inline int kill_cad_pid(int sig, int priv) · line 2582
Quoted 7 lines
include/linux/sched.h +4
-0@@ -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".
Quoted 7 lines
Quoted 6 lines
Anyway, I am a bit puzzled with this patch.
You say "unless user code deliberately set
SS_AUTODISARMwhen 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. :)
Quoted 14 lines
Quoted 7 lines
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 setSS_AUTODISARMand 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.Quoted 4 lines
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.Quoted 1 line
If this behavior would be problematic for you, can you explain why?
Quoted 8 lines
Quoted 5 lines
Only theoretically: if someone sets
SS_AUTODISARMinside 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
Quoted 124 lines · Stas Sergeev
14.05.2016 07:18, Andy Lutomirski пишет:
Quoted 112 lines · Stas Sergeev
Quoted 103 lines
09.05.2016 04:32, Andy Lutomirski пишет:
Quoted 65 lines · Stas Sergeev
Quoted 64 lines
03.05.2016 20:31, Andy Lutomirski пишет:
Quoted 51 lines
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 ifSS_AUTODISARMis 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 <[email protected]>Cc Al Viro <[email protected]>Cc Aleksa Sarai <[email protected]>Cc Amanieu d'Antras <[email protected]>Cc Andrea Arcangeli <[email protected]>Cc Andrew Morton <[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Denys Vlasenko <[email protected]>Cc Eric W. Biederman <[email protected]>Cc Frederic Weisbecker <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Heinrich Schuchardt <[email protected]>Cc Jason Low <[email protected]>Cc Josh Triplett <[email protected]>Cc Konstantin Khlebnikov <[email protected]>Cc Linus Torvalds <[email protected]>Cc Oleg Nesterov <[email protected]>Cc Palmer Dabbelt <[email protected]>Cc Paul Moore <[email protected]>Cc Pavel Emelyanov <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Richard Weinberger <[email protected]>Cc Sasha Levin <[email protected]>Cc Shuah Khan <[email protected]>Cc Tejun Heo <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Vladimir Davydov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- include/linux/sched.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)include/linux/sched.h +0
-0include/linux/sched.h static inline int kill_cad_pid(int sig, int priv) · line 2582
Quoted 7 lines
include/linux/sched.h +4
-0@@ -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".
Quoted 7 lines
Quoted 6 lines
Anyway, I am a bit puzzled with this patch.
You say "unless user code deliberately set
SS_AUTODISARMwhen 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. :)
Quoted 15 lines
Quoted 7 lines
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 setSS_AUTODISARMand 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.Quoted 4 lines
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.Quoted 7 lines
Quoted 1 line
If this behavior would be problematic for you, can you explain why?
Only theoretically: if someone sets
SS_AUTODISARMinside 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.Quoted 3 lines
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;
Quoted 7 lines
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
In reply to Andy LutomirskiThe handling for old kernels was wrong. Fix it.
--- tools/testing/selftests/sigaltstack/sas.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
tools/testing/selftests/sigaltstack/sas.c +14 -7
@@ -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,
Signature · 1 line
2.5.5
tip-bot for Andy Lutomirski
merge bot[tip:core/signals] selftests/sigaltstack: Fix the sigaltstack test on old kernels
selftests/sigaltstack: Fix the sigaltstack test on old kernels
Commit-ID: 158b67b5c5ccda9b909f18028a3cd17185ca1efd Gitweb: http://git.kernel.org/tip/158b67b5c5ccda9b909f18028a3cd17185ca1efd Author: Andy Lutomirski <[email protected]> AuthorDate: Tue, 3 May 2016 10:31:50 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 4 May 2016 08:34:13 +0200
selftests/sigaltstack: Fix the sigaltstack test on old kernels
The handling for old kernels was wrong, resulting in a segfault. Fix it.
--- tools/testing/selftests/sigaltstack/sas.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
tools/testing/selftests/sigaltstack/sas.c +14 -7
@@ -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,
Re: [PATCH 2/4] selftests/sigaltstack: Fix the sas test on old kernels
In reply to Andy Lutomirski03.05.2016 20:31, Andy Lutomirski пишет:
Quoted 45 lines
The handling for old kernels was wrong. Fix it.
Reported-by Ingo Molnar <[email protected]>Cc Stas Sergeev <[email protected]>Cc Al Viro <viro-RmSDqhL/[email protected]>Cc Andrew Morton <akpm-de/[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Denys Vlasenko <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Linus Torvalds <torvalds-de/[email protected]>Cc Oleg Nesterov <[email protected]>Cc Pavel Emelyanov <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Shuah Khan <[email protected]>Cc Thomas Gleixner <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- tools/testing/selftests/sigaltstack/sas.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)tools/testing/selftests/sigaltstack/sas.c +1
-5@@ -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) {
tools/testing/selftests/sigaltstack/sas.c int main(void) · line 128
Quoted 10 lines
tools/testing/selftests/sigaltstack/sas.c +8
-2-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
Quoted 61 lines · Stas Sergeev
03.05.2016 20:31, Andy Lutomirski пишет:
Quoted 44 lines
The handling for old kernels was wrong. Fix it.
Reported-by Ingo Molnar <[email protected]>Cc Stas Sergeev <[email protected]>Cc Al Viro <viro-RmSDqhL/[email protected]>Cc Andrew Morton <akpm-de/[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Denys Vlasenko <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Linus Torvalds <torvalds-de/[email protected]>Cc Oleg Nesterov <[email protected]>Cc Pavel Emelyanov <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Shuah Khan <[email protected]>Cc Thomas Gleixner <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- tools/testing/selftests/sigaltstack/sas.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)tools/testing/selftests/sigaltstack/sas.c +1
-5@@ -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) {tools/testing/selftests/sigaltstack/sas.c int main(void) · line 128
Quoted 10 lines
tools/testing/selftests/sigaltstack/sas.c +8
-2-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:
Quoted 9 lines
$ ./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()
In reply to Andy Lutomirskisigaltstack()'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.
--- kernel/signal.c | 3 ++- tools/testing/selftests/sigaltstack/sas.c | 19 ++++++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-)
kernel/signal.c +2 -1
@@ -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;
tools/testing/selftests/sigaltstack/sas.c +16 -3
@@ -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 ;
Signature · 1 line
2.5.5
Re: [PATCH 3/4] signals/sigaltstack: Report current flag bits in sigaltstack()
Quoted 8 lines · 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 <[email protected]>
This SOB seems stray as well, so I've removed it too.
Thanks,
Ingo
tip-bot for Andy Lutomirski
merge bot[tip:core/signals] signals/sigaltstack: Report current flag bits in sigaltstack()
In reply to Andy Lutomirskisignals/sigaltstack: Report current flag bits in sigaltstack()
Commit-ID: 0318bc8a919ded355eaa5078689924a15c1bf52a Gitweb: http://git.kernel.org/tip/0318bc8a919ded355eaa5078689924a15c1bf52a Author: Andy Lutomirski <[email protected]> AuthorDate: Tue, 3 May 2016 10:31:51 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 4 May 2016 08:34:14 +0200
signals/sigaltstack: Report current flag bits in sigaltstack()
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.
--- kernel/signal.c | 3 ++- tools/testing/selftests/sigaltstack/sas.c | 19 ++++++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-)
kernel/signal.c +2 -1
@@ -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;
tools/testing/selftests/sigaltstack/sas.c +16 -3
@@ -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 ;
PATCH 4/4 signals/sigaltstack: Change SS_AUTODISARM to (1U << 31)
In reply to Andy LutomirskiUsing 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.
--- include/uapi/linux/signal.h | 2 +- tools/testing/selftests/sigaltstack/sas.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
include/uapi/linux/signal.h +1 -1
@@ -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
tools/testing/selftests/sigaltstack/sas.c +1 -1
@@ -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;
Signature · 1 line
2.5.5
tip-bot for Andy Lutomirski
merge bot[tip:core/signals] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31)
signals/sigaltstack: Change SS_AUTODISARM to (1U << 31)
Commit-ID: 91c6180572e2fec71701d646ffc40ad30986275c Gitweb: http://git.kernel.org/tip/91c6180572e2fec71701d646ffc40ad30986275c Author: Andy Lutomirski <[email protected]> AuthorDate: Tue, 3 May 2016 10:31:52 -0700 Committer: Ingo Molnar <[email protected]> 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.
--- include/uapi/linux/signal.h | 2 +- tools/testing/selftests/sigaltstack/sas.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
include/uapi/linux/signal.h +1 -1
@@ -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
tools/testing/selftests/sigaltstack/sas.c +1 -1
@@ -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;
Re: [PATCH 4/4] signals/sigaltstack: Change SS_AUTODISARM to (1U << 31)
In reply to Andy Lutomirski03.05.2016 20:31, Andy Lutomirski пишет:
Quoted 46 lines
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 <[email protected]>Cc Al Viro <viro-RmSDqhL/[email protected]>Cc Aleksa Sarai <cyphar-gVpy/LI/[email protected]>Cc Amanieu d'Antras <[email protected]>Cc Andrea Arcangeli <[email protected]>Cc Andrew Morton <akpm-de/[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Denys Vlasenko <[email protected]>Cc Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>Cc Frederic Weisbecker <[email protected]>Cc H. Peter Anvin <[email protected]>Cc Heinrich Schuchardt <[email protected]>Cc Jason Low <[email protected]>Cc Josh Triplett <[email protected]>Cc Konstantin Khlebnikov <[email protected]>Cc Linus Torvalds <torvalds-de/[email protected]>Cc Oleg Nesterov <[email protected]>Cc Palmer Dabbelt <[email protected]>Cc Paul Moore <[email protected]>Cc Pavel Emelyanov <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Richard Weinberger <richard-/[email protected]>Cc Sasha Levin <[email protected]>Cc Shuah Khan <[email protected]>Cc Tejun Heo <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Vladimir Davydov <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- include/uapi/linux/signal.h | 2 +- tools/testing/selftests/sigaltstack/sas.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)include/uapi/linux/signal.h +0
-0
include/uapi/linux/signal.h line 11
Quoted 5 lines
include/uapi/linux/signal.h +1
-1@@ -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.
Quoted 3 lines · Andy Lutomirski
The first three are fixes IMO. The fourth changes the
SS_AUTODISARMbit. 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