[PATCH] s390/entry: add support for syscall stack randomization

[PATCH] s390/entry: add support for syscall stack randomization

[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(&regs->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

Re: [PATCH] s390/entry: add support for syscall stack randomization

[Heiko Carstens]

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!

Re: [PATCH] s390/entry: add support for syscall stack randomization

[Kees Cook]

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(&regs->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

Re: [PATCH] s390/entry: add support for syscall stack randomization

[Sven Schnelle]

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.

Re: [PATCH] s390/entry: add support for syscall stack randomization

[Sven Schnelle]

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.

Re: [PATCH] s390/entry: add support for syscall stack randomization

[Kees Cook]

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