Date: Tue, 30 Nov 2021 14:23:16 +0000 From: Andrew Turner <andrew@freebsd.org> To: Jessica Clarke <jrtc27@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: ae92ace05fd4 - main - Per-thread stack canary on arm64 Message-ID: <BB458EDA-2F97-4EFF-A8E4-08355417A712@freebsd.org> In-Reply-To: <F680BF76-857F-4676-857E-DB0186828DA0@freebsd.org> References: <202111261451.1AQEpJ7Y040922@gitrepo.freebsd.org> <F680BF76-857F-4676-857E-DB0186828DA0@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_CD84F761-7372-4DCD-A649-4391169FF8E4 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On 30 Nov 2021, at 13:30, Jessica Clarke <jrtc27@freebsd.org> wrote: >=20 > On 26 Nov 2021, at 14:51, Andrew Turner <andrew@FreeBSD.org = <mailto:andrew@FreeBSD.org>> wrote: >>=20 >> The branch main has been updated by andrew: >>=20 >> URL: = https://cgit.FreeBSD.org/src/commit/?id=3Dae92ace05fd4fcf64e3bb787951578f6= 55b1fa5f >>=20 >> 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 >>=20 >> Per-thread stack canary on arm64 >>=20 >> 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. >>=20 >> 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. >>=20 >> 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(+) >>=20 >> 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 =3D=3D 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)); >>=20 >> 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 >>=20 >> +#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 >>=20 >> @@ -200,6 +207,13 @@ mp_virtdone: >> ldr x4, [x4] >> mov sp, x4 >>=20 >> +#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 =3D = ARM64_BUS_NONE; >> */ >> struct pcpu pcpu0; >>=20 >> +#if defined(PERTHREAD_SSP) >> +/* >> + * The boot SSP canary. Will be replaced with a per-thread canary = when >> + * scheduling has started. >> + */ >> +uintptr_t boot_canary =3D 0x49a2d892bc05a0b1ul; >=20 > Is it *really* a pointer? That sure looks like it=E2=80=99s really a = size_t or > unsigned long. I doubt you=E2=80=99d want a capability on CHERI (well, = you=E2=80=99d > turn the feature off because it=E2=80=99s a waste of time there, but = still). >=20 > Jess It=E2=80=99s a uintptr_t in glibc. It=E2=80=99s also sitting beside = other pointers (the return address, etc) so any wasted space on the = stack would be from the size increase, not due to padding for alignment. Andrew --Apple-Mail=_CD84F761-7372-4DCD-A649-4391169FF8E4--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BB458EDA-2F97-4EFF-A8E4-08355417A712>