On 2 Feb 2023, at 16:48, Andrew Turner <and...@freebsd.org> wrote:
>
> The branch main has been updated by andrew:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=f29942229d24ebb8b98f8c5d02f3c8632648007e
>
> commit f29942229d24ebb8b98f8c5d02f3c8632648007e
> Author: Andrew Turner <and...@freebsd.org>
> AuthorDate: 2023-01-25 17:47:39 +0000
> Commit: Andrew Turner <and...@freebsd.org>
> CommitDate: 2023-02-02 16:43:15 +0000
>
> Read the arm64 far early in el0 exceptions
>
> 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.
>
> Fix this be always reading the fault address before dereferencing the
> current thread pointer.
>
> 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(-)
>
> 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)
>
> 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$");
>
> /* 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)
> }
>
> 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;
>
> /* Check we have a sane environment when entering from userland */
> @@ -573,27 +573,15 @@ do_el0_sync(struct thread *td, struct trapframe *frame)
>
> esr = frame->tf_esr;
> exception = ESR_ELx_EXCEPTION(esr);
> - switch (exception) {
> - case EXCP_INSN_ABORT_L:
> - far = READ_SPECIALREG(far_el1);
> -
> + if (exception == 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 = PCPU_GET(bp_harden);
> - if (bp_harden != NULL)
> - bp_harden();
> - }
> - break;
> - case EXCP_UNKNOWN:
> - case EXCP_DATA_ABORT_L:
> - case EXCP_DATA_ABORT:
> - case EXCP_WATCHPT_EL0:
> - far = READ_SPECIALREG(far_el1);
> - break;
> + bp_harden = PCPU_GET(bp_harden);
> + if (bp_harden != NULL)
> + bp_harden();
> }
> intr_enable();
>
