From: Michael Ellerman <mpe@ellerman•id.au>
To: Daniel Axtens <dja@axtens•net>, linuxppc-dev@ozlabs•org
Cc: npiggin@gmail•com
Subject: Re: [PATCH v5 2/2] powerpc/64: Prevent stack protection in early boot
Date: Thu, 19 Mar 2020 15:00:18 +1100 [thread overview]
Message-ID: <87eetpdmel.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <87wo7hn8az.fsf@dja-thinkpad.axtens.net>
Daniel Axtens <dja@axtens•net> writes:
> Michael Ellerman <mpe@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
prev parent reply other threads:[~2020-03-19 4:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-16 12:44 [PATCH v5 1/2] powerpc/64: Setup a paca before parsing device tree etc Michael Ellerman
2020-03-16 12:44 ` [PATCH v5 2/2] powerpc/64: Prevent stack protection in early boot Michael Ellerman
2020-03-18 12:42 ` Daniel Axtens
2020-03-19 4:00 ` Michael Ellerman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87eetpdmel.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=dja@axtens$(echo .)net \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=npiggin@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox