Date: Thu, 2 Feb 2023 21:00:57 +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: <0E3AAA1E-0B3E-4C4F-A425-CEE13BAE8723@freebsd.org> In-Reply-To: <202302021648.312GmSXI049747@gitrepo.freebsd.org> References: <202302021648.312GmSXI049747@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 *); This did not address my feedback regarding EL1 debug exceptions also clobbering FAR. 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(); >=20
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0E3AAA1E-0B3E-4C4F-A425-CEE13BAE8723>