Re: [PATCH] powerpc: Interrupt handler stack randomisation

["Nicholas Piggin" <npiggin@gmail.com>]

On Tue Oct 25, 2022 at 1:38 PM AEST, Rohan McLure wrote:
> Stack frames used by syscall handlers support random offsets as of
> commit f4a0318f278d (powerpc: add support for syscall stack randomization).
> Implement the same for general interrupt handlers, by applying the
> random stack offset and then updating this offset from within the
> DEFINE_INTERRUPT_HANDLER macros.
>
> Applying this offset perturbs the layout of interrupt handler stack
> frames, rendering to the kernel stack more difficult to control by means
> of user invoked interrupts.

Can you randomise the irq/softirq stacks as well with a patch 2?

_Probably_ don't have to do another mftb for those AFAICS. Hard irq
should be driven by one of these handlers (including in the irq replay
case). Softirq fast path is driven from irq_exit() from the async
irq handlers, so that should be pretty well randomized. Softirq
slowpath is done by ksoftirqd using its own stack, so nothing to
be done there.

>
> Link: https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-May/243238.html
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/interrupt.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 4745bb9998bd..b7f7beff4e13 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -68,6 +68,7 @@
>  
>  #include <linux/context_tracking.h>
>  #include <linux/hardirq.h>
> +#include <linux/randomize_kstack.h>
>  #include <asm/cputime.h>
>  #include <asm/firmware.h>
>  #include <asm/ftrace.h>
> @@ -448,9 +449,12 @@ interrupt_handler long func(struct pt_regs *regs)			\
>  	long ret;							\
>  									\
>  	__hard_RI_enable();						\
> +	add_random_kstack_offset();					\
>  									\
>  	ret = ____##func (regs);					\
>  									\
> +	choose_random_kstack_offset(mftb());				\
> +									\
>  	return ret;							\
>  }									\
>  NOKPROBE_SYMBOL(func);							\
> @@ -480,9 +484,11 @@ static __always_inline void ____##func(struct pt_regs *regs);		\
>  interrupt_handler void func(struct pt_regs *regs)			\
>  {									\
>  	interrupt_enter_prepare(regs);					\
> +	add_random_kstack_offset();					\
>  									\
>  	____##func (regs);						\
>  									\
> +	choose_random_kstack_offset(mftb());				\
>  	interrupt_exit_prepare(regs);					\
>  }									\
>  NOKPROBE_SYMBOL(func);							\

Hmm. It uses per-cpu data, so it actually can't be used for all
interrupt types, HMI and MCE because they're real-mode. SLB because
that might take an SLB fault, and maybe not hash faults either because
they might fault again I think?

You'd have to change the core code to let us put the offset in the paca.
Not sure if 32-bit has any such restrictions (e.g., does 32s need
MSR[RI] enabled before accessing per-cpu data?)

If you can do that that then I'd also put it as the first thing in
the func(), because the enter_prepare code can call non-trivial code
(e.g., irq_enter / nmi_enter).

Thnaks,
Nick