Date: Tue, 14 Feb 2023 02:57:53 +0000 From: Jessica Clarke <jrtc27@freebsd.org> To: Andrew Turner <andrew@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: f29942229d24 - main - Read the arm64 far early in el0 exceptions Message-ID: <1F46E64A-2261-47B1-BA73-3649DB1E08CE@freebsd.org> In-Reply-To: <0E3AAA1E-0B3E-4C4F-A425-CEE13BAE8723@freebsd.org> References: <202302021648.312GmSXI049747@gitrepo.freebsd.org> <0E3AAA1E-0B3E-4C4F-A425-CEE13BAE8723@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2 Feb 2023, at 21:00, Jessica Clarke <jrtc27@FreeBSD.org> wrote: >=20 > On 2 Feb 2023, at 16:48, Andrew Turner <andrew@FreeBSD.org> wrote: >>=20 >> The branch main has been updated by andrew: >>=20 >> URL: = https://cgit.FreeBSD.org/src/commit/?id=3Df29942229d24ebb8b98f8c5d02f3c863= 2648007e >>=20 >> commit f29942229d24ebb8b98f8c5d02f3c8632648007e >> Author: Andrew Turner <andrew@FreeBSD.org> >> AuthorDate: 2023-01-25 17:47:39 +0000 >> Commit: Andrew Turner <andrew@FreeBSD.org> >> CommitDate: 2023-02-02 16:43:15 +0000 >>=20 >> Read the arm64 far early in el0 exceptions >>=20 >> When handling userspace exceptions on arm64 we need to dereference = the >> current thread pointer. If this is being promoted/demoted there is = a >> small window where it will cause another exception to be hit. As = this >> second exception will set the fault address register we will read = the >> incorrect value in the userspace exception handler. >>=20 >> Fix this be always reading the fault address before dereferencing = the >> current thread pointer. >>=20 >> Reported by: olivier@ >> Reviewed by: markj >> Sponsored by: Arm Ltd >> Differential Revision: https://reviews.freebsd.org/D38196 >> --- >> sys/arm64/arm64/exception.S | 15 +++++++++++++++ >> sys/arm64/arm64/trap.c | 26 +++++++------------------- >> 2 files changed, 22 insertions(+), 19 deletions(-) >>=20 >> diff --git a/sys/arm64/arm64/exception.S = b/sys/arm64/arm64/exception.S >> index 4a74358afeb9..55bac5e5228a 100644 >> --- a/sys/arm64/arm64/exception.S >> +++ b/sys/arm64/arm64/exception.S >> @@ -212,10 +212,25 @@ ENTRY(handle_el1h_irq) >> END(handle_el1h_irq) >>=20 >> ENTRY(handle_el0_sync) >> + /* >> + * Read the fault address early. The current thread structure = may >> + * be transiently unmapped if it is part of a memory range being >> + * promoted or demoted to/from a superpage. As this involves a >> + * break-before-make sequence there is a short period of time = where >> + * an access will raise an exception. If this happens the fault >> + * address will be changed to the kernel address so a later read = of >> + * far_el1 will give the wrong value. >> + * >> + * The earliest memory access that could trigger a fault is in a >> + * function called by the save_registers macro so this is the = latest >> + * we can read the userspace value. >> + */ >> + mrs x19, far_el1 >> save_registers 0 >> ldr x0, [x18, #PC_CURTHREAD] >> mov x1, sp >> str x1, [x0, #TD_FRAME] >> + mov x2, x19 >> bl do_el0_sync >> do_ast >> restore_registers 0 >> diff --git a/sys/arm64/arm64/trap.c b/sys/arm64/arm64/trap.c >> index 4e54a06548cc..1b33d7aa60c4 100644 >> --- a/sys/arm64/arm64/trap.c >> +++ b/sys/arm64/arm64/trap.c >> @@ -76,7 +76,7 @@ __FBSDID("$FreeBSD$"); >>=20 >> /* Called from exception.S */ >> void do_el1h_sync(struct thread *, struct trapframe *); >=20 > This did not address my feedback regarding EL1 debug exceptions also > clobbering FAR. Ping, now after this has been MFC=E2=80=99ed without so much as a reply = to my feedback here nor on the Phabricator review. Jess >> -void do_el0_sync(struct thread *, struct trapframe *); >> +void do_el0_sync(struct thread *, struct trapframe *, uint64_t far); >> void do_el0_error(struct trapframe *); >> void do_serror(struct trapframe *); >> void unhandled_exception(struct trapframe *); >> @@ -559,11 +559,11 @@ do_el1h_sync(struct thread *td, struct = trapframe *frame) >> } >>=20 >> void >> -do_el0_sync(struct thread *td, struct trapframe *frame) >> +do_el0_sync(struct thread *td, struct trapframe *frame, uint64_t = far) >> { >> pcpu_bp_harden bp_harden; >> uint32_t exception; >> - uint64_t esr, far; >> + uint64_t esr; >> int dfsc; >>=20 >> /* Check we have a sane environment when entering from userland = */ >> @@ -573,27 +573,15 @@ do_el0_sync(struct thread *td, struct trapframe = *frame) >>=20 >> esr =3D frame->tf_esr; >> exception =3D ESR_ELx_EXCEPTION(esr); >> - switch (exception) { >> - case EXCP_INSN_ABORT_L: >> - far =3D READ_SPECIALREG(far_el1); >> - >> + if (exception =3D=3D EXCP_INSN_ABORT_L && far > = VM_MAXUSER_ADDRESS) { >> /* >> * Userspace may be trying to train the branch predictor = to >> * attack the kernel. If we are on a CPU affected by = this >> * call the handler to clear the branch predictor state. >> */ >> - if (far > VM_MAXUSER_ADDRESS) { >> - bp_harden =3D PCPU_GET(bp_harden); >> - if (bp_harden !=3D NULL) >> - bp_harden(); >> - } >> - break; >> - case EXCP_UNKNOWN: >> - case EXCP_DATA_ABORT_L: >> - case EXCP_DATA_ABORT: >> - case EXCP_WATCHPT_EL0: >> - far =3D READ_SPECIALREG(far_el1); >> - break; >> + bp_harden =3D PCPU_GET(bp_harden); >> + if (bp_harden !=3D NULL) >> + bp_harden(); >> } >> intr_enable();
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1F46E64A-2261-47B1-BA73-3649DB1E08CE>