[PATCH v5 2/2] powerpc/64: Prevent stack protection in early boot

[Daniel Axtens]

Michael Ellerman <mpe at ellerman.id.au> writes:

> The previous commit reduced the amount of code that is run before we
> setup a paca. However there are still a few remaining functions that
> run with no paca, or worse, with an arbitrary value in r13 that will
> be used as a paca pointer.
>
> In particular the stack protector canary is stored in the paca, so if
> stack protector is activated for any of these functions we will read
> the stack canary from wherever r13 points. If r13 happens to point
> outside of memory we will get a machine check / checkstop.
>
> For example if we modify initialise_paca() to trigger stack
> protection, and then boot in the mambo simulator with r13 poisoned in
> skiboot before calling the kernel:
>
>   DEBUG: 19952232: (19952232): INSTRUCTION: PC=0xC0000000191FC1E8: [0x3C4C006D]: addis   r2,r12,0x6D [fetch]
>   DEBUG: 19952236: (19952236): INSTRUCTION: PC=0xC00000001807EAD8: [0x7D8802A6]: mflr    r12 [fetch]
>   FATAL ERROR: 19952276: (19952276): Check Stop for 0:0: Machine Check with ME bit of MSR off
>   DEBUG: 19952276: (19952276): INSTRUCTION: PC=0xC0000000191FCA7C: [0xE90D0CF8]: ld      r8,0xCF8(r13) [Instruction Failed]
>   INFO: 19952276: (19952277): ** Execution stopped: Mambo Error, Machine Check Stop,  **
>   systemsim % bt
>   pc:                             0xC0000000191FCA7C      initialise_paca+0x54
>   lr:                             0xC0000000191FC22C      early_setup+0x44
>   stack:0x00000000198CBED0        0x0     +0x0
>   stack:0x00000000198CBF00        0xC0000000191FC22C      early_setup+0x44
>   stack:0x00000000198CBF90        0x1801C968      +0x1801C968
>
> So annotate the relevant functions to ensure stack protection is never
> enabled for them.

This all makes sense to me, although I don't really understand the stack
protector especially well.

I have checked and I can find no other C functions that are called
before early_setup.

Do we need to do add setup_64.c to the part of the Makefile that
disables tracing of early boot?

ifdef CONFIG_FUNCTION_TRACER
# Do not trace early boot code
CFLAGS_REMOVE_cputable.o = $(CC_FLAGS_FTRACE)
CFLAGS_REMOVE_prom_init.o = $(CC_FLAGS_FTRACE)
CFLAGS_REMOVE_btext.o = $(CC_FLAGS_FTRACE)
CFLAGS_REMOVE_prom.o = $(CC_FLAGS_FTRACE)
-> should we add setup_64.c here?
endif

Regards,
Daniel


>
> Fixes: 06ec27aea9fc ("powerpc/64: add stack protector support")
> Cc: stable at vger.kernel.org # v4.20+
> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
> ---
>  arch/powerpc/kernel/paca.c     | 4 ++--
>  arch/powerpc/kernel/setup.h    | 2 ++
>  arch/powerpc/kernel/setup_64.c | 2 +-
>  3 files changed, 5 insertions(+), 3 deletions(-)
>
> v5: New.
>
> diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
> index 0ee6308541b1..3f91ccaa9c74 100644
> --- a/arch/powerpc/kernel/paca.c
> +++ b/arch/powerpc/kernel/paca.c
> @@ -176,7 +176,7 @@ static struct slb_shadow * __init new_slb_shadow(int cpu, unsigned long limit)
>  struct paca_struct **paca_ptrs __read_mostly;
>  EXPORT_SYMBOL(paca_ptrs);
>  
> -void __init initialise_paca(struct paca_struct *new_paca, int cpu)
> +void __init __nostackprotector initialise_paca(struct paca_struct *new_paca, int cpu)
>  {
>  #ifdef CONFIG_PPC_PSERIES
>  	new_paca->lppaca_ptr = NULL;
> @@ -205,7 +205,7 @@ void __init initialise_paca(struct paca_struct *new_paca, int cpu)
>  }
>  
>  /* Put the paca pointer into r13 and SPRG_PACA */
> -void setup_paca(struct paca_struct *new_paca)
> +void __nostackprotector setup_paca(struct paca_struct *new_paca)
>  {
>  	/* Setup r13 */
>  	local_paca = new_paca;
> diff --git a/arch/powerpc/kernel/setup.h b/arch/powerpc/kernel/setup.h
> index 2dd0d9cb5a20..d210671026e9 100644
> --- a/arch/powerpc/kernel/setup.h
> +++ b/arch/powerpc/kernel/setup.h
> @@ -8,6 +8,8 @@
>  #ifndef __ARCH_POWERPC_KERNEL_SETUP_H
>  #define __ARCH_POWERPC_KERNEL_SETUP_H
>  
> +#define __nostackprotector __attribute__((__optimize__("no-stack-protector")))
> +
>  void initialize_cache_info(void);
>  void irqstack_early_init(void);
>  
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 17886d147dd0..438a9befce41 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -279,7 +279,7 @@ void __init record_spr_defaults(void)
>   * device-tree is not accessible via normal means at this point.
>   */
>  
> -void __init early_setup(unsigned long dt_ptr)
> +void __init __nostackprotector early_setup(unsigned long dt_ptr)
>  {
>  	static __initdata struct paca_struct boot_paca;
>  
> -- 
> 2.24.1