* [PATCH] s390/entry: add support for syscall stack randomization
@ 2021-04-29 9:14 Sven Schnelle
2021-04-30 15:42 ` Heiko Carstens
2021-04-30 17:10 ` Kees Cook
0 siblings, 2 replies; 6+ messages in thread
From: Sven Schnelle @ 2021-04-29 9:14 UTC (permalink / raw
To: Heiko Carstens, Vasily Gorbik
Cc: Kees Cook, Thomas Gleixner, linux-s390, Sven Schnelle
This adds support for adding a random offset to the stack while handling
syscalls. The patch uses get_tod_clock_fast() as this is considered good
enough and has much less performance penalty compared to using
get_random_int(). The patch also adds randomization in pgm_check_handler()
as the sigreturn/rt_sigreturn system calls might be called from there.
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
arch/s390/Kconfig | 1 +
arch/s390/include/asm/entry-common.h | 10 ++++++++++
arch/s390/kernel/syscall.c | 1 +
arch/s390/kernel/traps.c | 2 ++
4 files changed, 14 insertions(+)
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index c1ff874e6c2e..1900428ce557 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -137,6 +137,7 @@ config S390
select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN
select HAVE_ARCH_KASAN_VMALLOC
+ select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
select HAVE_ARCH_SECCOMP_FILTER
select HAVE_ARCH_SOFT_DIRTY
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/s390/include/asm/entry-common.h b/arch/s390/include/asm/entry-common.h
index 9cceb26ed63f..baa8005090c3 100644
--- a/arch/s390/include/asm/entry-common.h
+++ b/arch/s390/include/asm/entry-common.h
@@ -4,9 +4,11 @@
#include <linux/sched.h>
#include <linux/audit.h>
+#include <linux/randomize_kstack.h>
#include <linux/tracehook.h>
#include <linux/processor.h>
#include <linux/uaccess.h>
+#include <asm/timex.h>
#include <asm/fpu/api.h>
#define ARCH_EXIT_TO_USER_MODE_WORK (_TIF_GUARDED_STORAGE | _TIF_PER_TRAP)
@@ -48,6 +50,14 @@ static __always_inline void arch_exit_to_user_mode(void)
#define arch_exit_to_user_mode arch_exit_to_user_mode
+static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
+ unsigned long ti_work)
+{
+ choose_random_kstack_offset(get_tod_clock_fast() & 0xff);
+}
+
+#define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
+
static inline bool on_thread_stack(void)
{
return !(((unsigned long)(current->stack) ^ current_stack_pointer()) & ~(THREAD_SIZE - 1));
diff --git a/arch/s390/kernel/syscall.c b/arch/s390/kernel/syscall.c
index bc8e650e377d..4e5cc7d2364e 100644
--- a/arch/s390/kernel/syscall.c
+++ b/arch/s390/kernel/syscall.c
@@ -142,6 +142,7 @@ void do_syscall(struct pt_regs *regs)
void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
{
+ add_random_kstack_offset();
enter_from_user_mode(regs);
memcpy(®s->gprs[8], S390_lowcore.save_area_sync, 8 * sizeof(unsigned long));
diff --git a/arch/s390/kernel/traps.c b/arch/s390/kernel/traps.c
index 63021d484626..8dd23c703718 100644
--- a/arch/s390/kernel/traps.c
+++ b/arch/s390/kernel/traps.c
@@ -17,6 +17,7 @@
#include "asm/ptrace.h"
#include <linux/kprobes.h>
#include <linux/kdebug.h>
+#include <linux/randomize_kstack.h>
#include <linux/extable.h>
#include <linux/ptrace.h>
#include <linux/sched.h>
@@ -301,6 +302,7 @@ void noinstr __do_pgm_check(struct pt_regs *regs)
unsigned int trapnr, syscall_redirect = 0;
irqentry_state_t state;
+ add_random_kstack_offset();
regs->int_code = *(u32 *)&S390_lowcore.pgm_ilc;
regs->int_parm_long = S390_lowcore.trans_exc_code;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] s390/entry: add support for syscall stack randomization
2021-04-29 9:14 [PATCH] s390/entry: add support for syscall stack randomization Sven Schnelle
@ 2021-04-30 15:42 ` Heiko Carstens
2021-04-30 17:10 ` Kees Cook
1 sibling, 0 replies; 6+ messages in thread
From: Heiko Carstens @ 2021-04-30 15:42 UTC (permalink / raw
To: Sven Schnelle; +Cc: Vasily Gorbik, Kees Cook, Thomas Gleixner, linux-s390
On Thu, Apr 29, 2021 at 11:14:51AM +0200, Sven Schnelle wrote:
> This adds support for adding a random offset to the stack while handling
> syscalls. The patch uses get_tod_clock_fast() as this is considered good
> enough and has much less performance penalty compared to using
> get_random_int(). The patch also adds randomization in pgm_check_handler()
> as the sigreturn/rt_sigreturn system calls might be called from there.
>
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
> arch/s390/Kconfig | 1 +
> arch/s390/include/asm/entry-common.h | 10 ++++++++++
> arch/s390/kernel/syscall.c | 1 +
> arch/s390/kernel/traps.c | 2 ++
> 4 files changed, 14 insertions(+)
Applied, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] s390/entry: add support for syscall stack randomization
2021-04-29 9:14 [PATCH] s390/entry: add support for syscall stack randomization Sven Schnelle
2021-04-30 15:42 ` Heiko Carstens
@ 2021-04-30 17:10 ` Kees Cook
2021-05-03 6:36 ` Sven Schnelle
1 sibling, 1 reply; 6+ messages in thread
From: Kees Cook @ 2021-04-30 17:10 UTC (permalink / raw
To: Sven Schnelle; +Cc: Heiko Carstens, Vasily Gorbik, Thomas Gleixner, linux-s390
On Thu, Apr 29, 2021 at 11:14:51AM +0200, Sven Schnelle wrote:
> This adds support for adding a random offset to the stack while handling
> syscalls. The patch uses get_tod_clock_fast() as this is considered good
Nice! :)
> enough and has much less performance penalty compared to using
> get_random_int(). The patch also adds randomization in pgm_check_handler()
> as the sigreturn/rt_sigreturn system calls might be called from there.
Ah, interesting. Is this path to syscalls unique to s390? (As in, should
x86 and arm64 gain coverage over a path that got missed?)
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
> arch/s390/Kconfig | 1 +
> arch/s390/include/asm/entry-common.h | 10 ++++++++++
> arch/s390/kernel/syscall.c | 1 +
> arch/s390/kernel/traps.c | 2 ++
> 4 files changed, 14 insertions(+)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index c1ff874e6c2e..1900428ce557 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -137,6 +137,7 @@ config S390
> select HAVE_ARCH_JUMP_LABEL_RELATIVE
> select HAVE_ARCH_KASAN
> select HAVE_ARCH_KASAN_VMALLOC
> + select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
> select HAVE_ARCH_SECCOMP_FILTER
> select HAVE_ARCH_SOFT_DIRTY
> select HAVE_ARCH_TRACEHOOK
> diff --git a/arch/s390/include/asm/entry-common.h b/arch/s390/include/asm/entry-common.h
> index 9cceb26ed63f..baa8005090c3 100644
> --- a/arch/s390/include/asm/entry-common.h
> +++ b/arch/s390/include/asm/entry-common.h
> @@ -4,9 +4,11 @@
>
> #include <linux/sched.h>
> #include <linux/audit.h>
> +#include <linux/randomize_kstack.h>
> #include <linux/tracehook.h>
> #include <linux/processor.h>
> #include <linux/uaccess.h>
> +#include <asm/timex.h>
> #include <asm/fpu/api.h>
>
> #define ARCH_EXIT_TO_USER_MODE_WORK (_TIF_GUARDED_STORAGE | _TIF_PER_TRAP)
> @@ -48,6 +50,14 @@ static __always_inline void arch_exit_to_user_mode(void)
>
> #define arch_exit_to_user_mode arch_exit_to_user_mode
>
> +static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
> + unsigned long ti_work)
> +{
> + choose_random_kstack_offset(get_tod_clock_fast() & 0xff);
What's the stack alignment on s390? Or, better question, what's the
expected number of entropy bits?
> +}
> +
> +#define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
> +
> static inline bool on_thread_stack(void)
> {
> return !(((unsigned long)(current->stack) ^ current_stack_pointer()) & ~(THREAD_SIZE - 1));
> diff --git a/arch/s390/kernel/syscall.c b/arch/s390/kernel/syscall.c
> index bc8e650e377d..4e5cc7d2364e 100644
> --- a/arch/s390/kernel/syscall.c
> +++ b/arch/s390/kernel/syscall.c
> @@ -142,6 +142,7 @@ void do_syscall(struct pt_regs *regs)
>
> void noinstr __do_syscall(struct pt_regs *regs, int per_trap)
> {
> + add_random_kstack_offset();
> enter_from_user_mode(regs);
>
> memcpy(®s->gprs[8], S390_lowcore.save_area_sync, 8 * sizeof(unsigned long));
> diff --git a/arch/s390/kernel/traps.c b/arch/s390/kernel/traps.c
> index 63021d484626..8dd23c703718 100644
> --- a/arch/s390/kernel/traps.c
> +++ b/arch/s390/kernel/traps.c
> @@ -17,6 +17,7 @@
> #include "asm/ptrace.h"
> #include <linux/kprobes.h>
> #include <linux/kdebug.h>
> +#include <linux/randomize_kstack.h>
> #include <linux/extable.h>
> #include <linux/ptrace.h>
> #include <linux/sched.h>
> @@ -301,6 +302,7 @@ void noinstr __do_pgm_check(struct pt_regs *regs)
> unsigned int trapnr, syscall_redirect = 0;
> irqentry_state_t state;
>
> + add_random_kstack_offset();
> regs->int_code = *(u32 *)&S390_lowcore.pgm_ilc;
> regs->int_parm_long = S390_lowcore.trans_exc_code;
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] s390/entry: add support for syscall stack randomization
2021-04-30 17:10 ` Kees Cook
@ 2021-05-03 6:36 ` Sven Schnelle
2021-05-03 9:13 ` Sven Schnelle
0 siblings, 1 reply; 6+ messages in thread
From: Sven Schnelle @ 2021-05-03 6:36 UTC (permalink / raw
To: Kees Cook; +Cc: Heiko Carstens, Vasily Gorbik, Thomas Gleixner, linux-s390
Hi Kees,
Kees Cook <keescook@chromium.org> writes:
> On Thu, Apr 29, 2021 at 11:14:51AM +0200, Sven Schnelle wrote:
>> enough and has much less performance penalty compared to using
>> get_random_int(). The patch also adds randomization in pgm_check_handler()
>> as the sigreturn/rt_sigreturn system calls might be called from there.
>
> Ah, interesting. Is this path to syscalls unique to s390? (As in, should
> x86 and arm64 gain coverage over a path that got missed?)
Yes, it's unique to s390. So there should be no need to do anything
similar on other architectures.
>> +static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
>> + unsigned long ti_work)
>> +{
>> + choose_random_kstack_offset(get_tod_clock_fast() & 0xff);
>
> What's the stack alignment on s390? Or, better question, what's the
> expected number of entropy bits?
The stack alignement on s390 is 8 bytes, so this should give us 5 bits
of entropy.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] s390/entry: add support for syscall stack randomization
2021-05-03 6:36 ` Sven Schnelle
@ 2021-05-03 9:13 ` Sven Schnelle
2021-05-03 18:31 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Sven Schnelle @ 2021-05-03 9:13 UTC (permalink / raw
To: Kees Cook; +Cc: Heiko Carstens, Vasily Gorbik, Thomas Gleixner, linux-s390
Hi Kees,
Sven Schnelle <svens@linux.ibm.com> writes:
> Kees Cook <keescook@chromium.org> writes:
>
>> On Thu, Apr 29, 2021 at 11:14:51AM +0200, Sven Schnelle wrote:
>>> enough and has much less performance penalty compared to using
>>> get_random_int(). The patch also adds randomization in pgm_check_handler()
>>> as the sigreturn/rt_sigreturn system calls might be called from there.
>>
>> Ah, interesting. Is this path to syscalls unique to s390? (As in, should
>> x86 and arm64 gain coverage over a path that got missed?)
>
> Yes, it's unique to s390. So there should be no need to do anything
> similar on other architectures.
I was a bit short with my reponse, so let me explain this a bit
further. On s390, when a signal handler needs to be called, we put a
'svc (system call) instruction on the Stack and set the address in the
register holding the return address (r14) to that address. That worked
fine until non-executable stacks where introduced. With non-executable
stacks, we get a program check instead when trying to execute the svc.
The kernel than checks whether the instruction that caused the fault
is the svc instruction, and if yes, it will redirect to the systemm call
code to execute the {rt_}sigreturn syscall. So we need to do the stack
offset randomization also in the program check handler to cover that path.
>
>>> +static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
>>> + unsigned long ti_work)
>>> +{
>>> + choose_random_kstack_offset(get_tod_clock_fast() & 0xff);
>>
>> What's the stack alignment on s390? Or, better question, what's the
>> expected number of entropy bits?
>
>
> The stack alignement on s390 is 8 bytes, so this should give us 5 bits
> of entropy.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] s390/entry: add support for syscall stack randomization
2021-05-03 9:13 ` Sven Schnelle
@ 2021-05-03 18:31 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2021-05-03 18:31 UTC (permalink / raw
To: Sven Schnelle; +Cc: Heiko Carstens, Vasily Gorbik, Thomas Gleixner, linux-s390
On Mon, May 03, 2021 at 11:13:01AM +0200, Sven Schnelle wrote:
> Hi Kees,
>
> Sven Schnelle <svens@linux.ibm.com> writes:
>
> > Kees Cook <keescook@chromium.org> writes:
> >
> >> On Thu, Apr 29, 2021 at 11:14:51AM +0200, Sven Schnelle wrote:
> >>> enough and has much less performance penalty compared to using
> >>> get_random_int(). The patch also adds randomization in pgm_check_handler()
> >>> as the sigreturn/rt_sigreturn system calls might be called from there.
> >>
> >> Ah, interesting. Is this path to syscalls unique to s390? (As in, should
> >> x86 and arm64 gain coverage over a path that got missed?)
> >
> > Yes, it's unique to s390. So there should be no need to do anything
> > similar on other architectures.
>
> I was a bit short with my reponse, so let me explain this a bit
> further. On s390, when a signal handler needs to be called, we put a
> 'svc (system call) instruction on the Stack and set the address in the
> register holding the return address (r14) to that address. That worked
> fine until non-executable stacks where introduced. With non-executable
> stacks, we get a program check instead when trying to execute the svc.
> The kernel than checks whether the instruction that caused the fault
> is the svc instruction, and if yes, it will redirect to the systemm call
> code to execute the {rt_}sigreturn syscall. So we need to do the stack
> offset randomization also in the program check handler to cover that path.
Ah-ha; thanks for the details! I appreciate it. :)
> >>> +static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
> >>> + unsigned long ti_work)
> >>> +{
> >>> + choose_random_kstack_offset(get_tod_clock_fast() & 0xff);
> >>
> >> What's the stack alignment on s390? Or, better question, what's the
> >> expected number of entropy bits?
> >
> >
> > The stack alignement on s390 is 8 bytes, so this should give us 5 bits
> > of entropy.
Sounds good!
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-03 18:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-29 9:14 [PATCH] s390/entry: add support for syscall stack randomization Sven Schnelle
2021-04-30 15:42 ` Heiko Carstens
2021-04-30 17:10 ` Kees Cook
2021-05-03 6:36 ` Sven Schnelle
2021-05-03 9:13 ` Sven Schnelle
2021-05-03 18:31 ` Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.