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

Daniel Axtens [off-list ref] writes:

Michael Ellerman [off-list ref] writes:

quoted

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.

The key details for this bug are that 1) some functions get stack
protection, if they have on-stack buffers etc. 2) that stack protection
involves reading a canary from the paca.

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

Thanks.

Except for all of prom_init.c but that's already built with no stack
protector.

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

No I don't think so.

Tracing is less of a concern during very early boot because although the
functions may be built to support tracing, you can't actually turn
tracing *on* that early.

Also setup_64.c is not purely early boot code, there are some functions
in there we would like to be able to trace.

As I was saying the other day, we may want to create a specific
directory (or file) for all the really early boot code where we turn off
all special options like tracing, kcov, stack protector etc.

cheers