Re: git: ae92ace05fd4 - main - Per-thread stack canary on arm64

On 26 Nov 2021, at 14:51, Andrew Turner <andrew@FreeBSD.org> wrote:
> 
> The branch main has been updated by andrew:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=ae92ace05fd4fcf64e3bb787951578f655b1fa5f
> 
> commit ae92ace05fd4fcf64e3bb787951578f655b1fa5f
> Author:     Andrew Turner <andrew@FreeBSD.org>
> AuthorDate: 2021-11-22 15:20:51 +0000
> Commit:     Andrew Turner <andrew@FreeBSD.org>
> CommitDate: 2021-11-26 14:44:00 +0000
> 
>    Per-thread stack canary on arm64
> 
>    With the update to llvm 13 we are able to tell the compiler it can find
>    the SSP canary relative to the register that holds the userspace stack
>    pointer. As this is unused in most of the kernel it can be used here
>    to point to a per-thread SSP canary.
> 
>    As the kernel could be built with an old toolchain, e.g. when upgrading
>    from 13, add a warning that the options was enabled but the compiler
>    doesn't support it to both the build and kernel boot.
> 
>    Discussed with: emaste
>    Sponsored by:   The FreeBSD Foundation
>    Differential Revision: https://reviews.freebsd.org/D33079
> ---
> sys/arm64/arm64/exception.S  |  7 +++++++
> sys/arm64/arm64/genassym.c   |  1 +
> sys/arm64/arm64/locore.S     | 14 ++++++++++++++
> sys/arm64/arm64/machdep.c    | 22 ++++++++++++++++++++++
> sys/arm64/arm64/pmap.c       |  4 ++++
> sys/arm64/arm64/vm_machdep.c | 10 ++++++++++
> sys/arm64/conf/std.arm64     |  1 +
> sys/arm64/include/proc.h     |  1 +
> sys/conf/Makefile.arm64      | 14 ++++++++++++++
> sys/conf/options.arm64       |  4 ++++
> 10 files changed, 78 insertions(+)
> 
> diff --git a/sys/arm64/arm64/exception.S b/sys/arm64/arm64/exception.S
> index 22f6b7ce6145..d81bbce0efc7 100644
> --- a/sys/arm64/arm64/exception.S
> +++ b/sys/arm64/arm64/exception.S
> @@ -66,6 +66,13 @@ __FBSDID("$FreeBSD$");
> 	mrs	x18, tpidr_el1
> 	add	x29, sp, #(TF_SIZE)
> .if \el == 0
> +#if defined(PERTHREAD_SSP)
> +	/* Load the SSP canary to sp_el0 */
> +	ldr	x1, [x18, #(PC_CURTHREAD)]
> +	add	x1, x1, #(TD_MD_CANARY)
> +	msr	sp_el0, x1
> +#endif
> +
> 	/* Apply the SSBD (CVE-2018-3639) workaround if needed */
> 	ldr	x1, [x18, #PC_SSBD]
> 	cbz	x1, 1f
> diff --git a/sys/arm64/arm64/genassym.c b/sys/arm64/arm64/genassym.c
> index 1575a0158dec..8e3ddc48317b 100644
> --- a/sys/arm64/arm64/genassym.c
> +++ b/sys/arm64/arm64/genassym.c
> @@ -73,6 +73,7 @@ ASSYM(TD_PCB, offsetof(struct thread, td_pcb));
> ASSYM(TD_FLAGS, offsetof(struct thread, td_flags));
> ASSYM(TD_FRAME, offsetof(struct thread, td_frame));
> ASSYM(TD_LOCK, offsetof(struct thread, td_lock));
> +ASSYM(TD_MD_CANARY, offsetof(struct thread, td_md.md_canary));
> 
> ASSYM(TF_SIZE, sizeof(struct trapframe));
> ASSYM(TF_SP, offsetof(struct trapframe, tf_sp));
> diff --git a/sys/arm64/arm64/locore.S b/sys/arm64/arm64/locore.S
> index 92415aab1555..bc9a7271e93a 100644
> --- a/sys/arm64/arm64/locore.S
> +++ b/sys/arm64/arm64/locore.S
> @@ -116,6 +116,13 @@ virtdone:
> 	cmp	x15, x14
> 	b.lo	1b
> 
> +#if defined(PERTHREAD_SSP)
> +	/* Set sp_el0 to the boot canary for early per-thread SSP to work */
> +	adrp	x15, boot_canary
> +	add	x15, x15, :lo12:boot_canary
> +	msr	sp_el0, x15
> +#endif
> +
> 	/* Backup the module pointer */
> 	mov	x1, x0
> 
> @@ -200,6 +207,13 @@ mp_virtdone:
> 	ldr	x4, [x4]
> 	mov	sp, x4
> 
> +#if defined(PERTHREAD_SSP)
> +	/* Set sp_el0 to the boot canary for early per-thread SSP to work */
> +	adrp	x15, boot_canary
> +	add	x15, x15, :lo12:boot_canary
> +	msr	sp_el0, x15
> +#endif
> +
> 	/* Load the kernel ttbr0 pagetable */
> 	msr	ttbr0_el1, x27
> 	isb
> diff --git a/sys/arm64/arm64/machdep.c b/sys/arm64/arm64/machdep.c
> index 59a634f4d30c..821d9ba19022 100644
> --- a/sys/arm64/arm64/machdep.c
> +++ b/sys/arm64/arm64/machdep.c
> @@ -109,6 +109,14 @@ enum arm64_bus arm64_bus_method = ARM64_BUS_NONE;
>  */
> struct pcpu pcpu0;
> 
> +#if defined(PERTHREAD_SSP)
> +/*
> + * The boot SSP canary. Will be replaced with a per-thread canary when
> + * scheduling has started.
> + */
> +uintptr_t boot_canary = 0x49a2d892bc05a0b1ul;

Is it *really* a pointer? That sure looks like it’s really a size_t or
unsigned long. I doubt you’d want a capability on CHERI (well, you’d
turn the feature off because it’s a waste of time there, but still).

Jess