arm64: Disable per-thread stack-smashing protection in data_abort()

With PERTHREAD_SSP configured, the compiler's stack-smashing protection
uses a per-thread canary value instead of a global value.  The value is
stored in td->td_md.md_canary; the sp_el0 register always contains a
pointer to that value, and certain functions selected by the compiler
will store the canary value on the stack as a part of the function
prologue (and will verify the copy as part of the epilogue).  In
particular, the thread structure may be accessed.

This happens to occur in data_abort(), which leads to the same problem
addressed by commit 2c10be9e06 ("arm64: Handle translation faults for
thread structures").  This commit fixes that directly, by disabling SSP
in data_abort() and a couple of related functions by using a function
attribute.  It also moves the update of sp_el0 out of C code in case
the compiler decides to start checking the canary in pmap_switch()
someday.

A different solution might be to move the canary value to the PCB, which
currently lives on the kernel stack and isn't subject to the same
problem as thread structures (if only because guard pages inhibit
superpage promotion).  However, there isn't any particular reason the
PCB has to live on the stack today; on amd64 it is embedded in struct
thread, reintroducing the same problem.  Keeping the reference canary
value at the top of the stack is also rather dubious since it could be
clobbered by a sufficiently large stack overflow.

A third solution could be to go back to the approach of commit
5aa5420ff2, and modify UMA to use the direct map for thread structures
even if KASAN is enabled.  But, transient promotions and demotions in
the direct map are possible too.

Reviewed by:	alc, kib, andrew
MFC after:	1 month
Sponsored by:	Juniper Networks, Inc.
Sponsored by:	Klara, Inc.
Differential Revision:	https://reviews.freebsd.org/D37255
This commit is contained in:
Mark Johnston 2022-11-07 15:53:41 -05:00
parent 4232f36eda
commit 03bf40c5d8
5 changed files with 44 additions and 7 deletions

View File

@ -1647,8 +1647,11 @@ pmap_extract_and_hold(pmap_t pmap, vm_offset_t va, vm_prot_t prot)
* Walks the page tables to translate a kernel virtual address to a
* physical address. Returns true if the kva is valid and stores the
* physical address in pa if it is not NULL.
*
* See the comment above data_abort() for the rationale for specifying
* NO_PERTHREAD_SSP here.
*/
bool
bool NO_PERTHREAD_SSP
pmap_klookup(vm_offset_t va, vm_paddr_t *pa)
{
pt_entry_t *pte, tpte;
@ -7052,10 +7055,6 @@ pmap_switch(struct thread *new)
/* Store the new curthread */
PCPU_SET(curthread, new);
#if defined(PERTHREAD_SSP)
/* Set the new threads SSP canary */
__asm("msr sp_el0, %0" :: "r"(&new->td_md.md_canary));
#endif
/* And the new pcb */
pcb = new->td_pcb;

View File

@ -80,9 +80,17 @@ ENTRY(cpu_throw)
/* This returns the thread pointer so no need to save it */
bl ptrauth_switch
#ifdef PERTHREAD_SSP
mov x19, x0
#endif
/* This returns the thread pcb */
bl pmap_switch
mov x4, x0
#ifdef PERTHREAD_SSP
/* Update the per-thread stack canary pointer. */
add x19, x19, #(TD_MD_CANARY)
msr sp_el0, x19
#endif
/* If we are single stepping, enable it */
ldr w5, [x4, #PCB_FLAGS]
@ -159,6 +167,11 @@ ENTRY(cpu_switch)
mov x2, x21
mov x1, x20
mov x0, x19
#ifdef PERTHREAD_SSP
/* Update the per-thread stack canary pointer. */
add x20, x20, #(TD_MD_CANARY)
msr sp_el0, x20
#endif
/*
* Release the old thread.

View File

@ -241,7 +241,13 @@ external_abort(struct thread *td, struct trapframe *frame, uint64_t esr,
panic("Unhandled EL%d external data abort", lower ? 0: 1);
}
static void
/*
* It is unsafe to access the stack canary value stored in "td" until
* kernel map translation faults are handled, see the pmap_klookup() call below.
* Thus, stack-smashing detection with per-thread canaries must be disabled in
* this function.
*/
static void NO_PERTHREAD_SSP
data_abort(struct thread *td, struct trapframe *frame, uint64_t esr,
uint64_t far, int lower)
{
@ -449,7 +455,10 @@ fpe_trap(struct thread *td, void *addr, uint32_t exception)
}
#endif
void
/*
* See the comment above data_abort().
*/
void NO_PERTHREAD_SSP
do_el1h_sync(struct thread *td, struct trapframe *frame)
{
uint32_t exception;

View File

@ -105,6 +105,12 @@
#define KSTACK_GUARD_PAGES 1 /* pages of kstack guard; 0 disables */
#define PCPU_PAGES 1
#ifdef PERTHREAD_SSP
#define NO_PERTHREAD_SSP __nostackprotector
#else
#define NO_PERTHREAD_SSP
#endif
/*
* Mach derived conversion macros
*/

View File

@ -896,6 +896,16 @@
#define __nosanitizethread
#endif
/*
* Make it possible to opt out of stack smashing protection.
*/
#if __has_attribute(no_stack_protector)
#define __nostackprotector __attribute__((no_stack_protector))
#else
#define __nostackprotector \
__attribute__((__optimize__("-fno-stack-protector")))
#endif
/* Guard variables and structure members by lock. */
#define __guarded_by(x) __lock_annotate(guarded_by(x))
#define __pt_guarded_by(x) __lock_annotate(pt_guarded_by(x))