Daniel Axtens <d...@axtens.net> writes: > Michael Ellerman <m...@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.
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
