From 2c10be9e06d4577ad8b60a7cb415ecbad60b5370 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Wed, 2 Nov 2022 13:27:27 -0400 Subject: [PATCH] arm64: Handle translation faults for thread structures The break-before-make requirement poses a problem when promoting or demoting mappings containing thread structures: a CPU may raise a translation fault while accessing curthread, and data_abort() accesses the thread again before pmap_fault() can translate the address and return. Normally this isn't a problem because we have a hack to ensure that slabs used by the thread zone are always accessed via the direct map, where promotions and demotions are rare. However, this hack doesn't work properly with UMA_MD_SMALL_ALLOC disabled, as is the case with KASAN configured (since our KASAN implementation does not shadow the direct map and so tries to force the use of the kernel map wherever possible). Fix the problem by modifying data_abort() to handle translation faults in the kernel map without dereferencing "td", i.e., curthread, and without enabling interrupts. pmap_klookup() has special handling for translation faults which makes it safe to call in this context. Then, revert the aforementioned hack. Reviewed by: kevans, alc, kib, andrew MFC after: 1 month Sponsored by: Juniper Networks, Inc. Sponsored by: Klara, Inc. Differential Revision: https://reviews.freebsd.org/D37231 --- sys/arm64/arm64/trap.c | 55 +++++++++++++++++++++++++++--------------- sys/kern/kern_thread.c | 14 +---------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/sys/arm64/arm64/trap.c b/sys/arm64/arm64/trap.c index 21843694f48c..f48e33db914e 100644 --- a/sys/arm64/arm64/trap.c +++ b/sys/arm64/arm64/trap.c @@ -246,7 +246,6 @@ data_abort(struct thread *td, struct trapframe *frame, uint64_t esr, uint64_t far, int lower) { struct vm_map *map; - struct proc *p; struct pcb *pcb; vm_prot_t ftype; int error, sig, ucode; @@ -268,28 +267,44 @@ data_abort(struct thread *td, struct trapframe *frame, uint64_t esr, } #endif - pcb = td->td_pcb; - p = td->td_proc; - if (lower) - map = &p->p_vmspace->vm_map; - else { - intr_enable(); - + if (lower) { + map = &td->td_proc->p_vmspace->vm_map; + } else if (!ADDR_IS_CANONICAL(far)) { /* We received a TBI/PAC/etc. fault from the kernel */ - if (!ADDR_IS_CANONICAL(far)) { - error = KERN_INVALID_ADDRESS; - goto bad_far; + error = KERN_INVALID_ADDRESS; + goto bad_far; + } else if (ADDR_IS_KERNEL(far)) { + /* + * Handle a special case: the data abort was caused by accessing + * a thread structure while its mapping was being promoted or + * demoted, as a consequence of the break-before-make rule. It + * is not safe to enable interrupts or dereference "td" before + * this case is handled. + * + * In principle, if pmap_klookup() fails, there is no need to + * call pmap_fault() below, but avoiding that call is not worth + * the effort. + */ + if (ESR_ELx_EXCEPTION(esr) == EXCP_DATA_ABORT) { + switch (esr & ISS_DATA_DFSC_MASK) { + case ISS_DATA_DFSC_TF_L0: + case ISS_DATA_DFSC_TF_L1: + case ISS_DATA_DFSC_TF_L2: + case ISS_DATA_DFSC_TF_L3: + if (pmap_klookup(far, NULL)) + return; + break; + } } - - /* The top bit tells us which range to use */ - if (ADDR_IS_KERNEL(far)) { + intr_enable(); + map = kernel_map; + } else { + intr_enable(); + map = &td->td_proc->p_vmspace->vm_map; + if (map == NULL) map = kernel_map; - } else { - map = &p->p_vmspace->vm_map; - if (map == NULL) - map = kernel_map; - } } + pcb = td->td_pcb; /* * Try to handle translation, access flag, and permission faults. @@ -334,11 +349,11 @@ data_abort(struct thread *td, struct trapframe *frame, uint64_t esr, /* Fault in the page. */ error = vm_fault_trap(map, far, ftype, VM_FAULT_NORMAL, &sig, &ucode); if (error != KERN_SUCCESS) { -bad_far: if (lower) { call_trapsignal(td, sig, ucode, (void *)far, ESR_ELx_EXCEPTION(esr)); } else { +bad_far: if (td->td_intr_nesting_level == 0 && pcb->pcb_onfault != 0) { frame->tf_x[0] = error; diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c index 39bda326dc0d..03fb20bd81d9 100644 --- a/sys/kern/kern_thread.c +++ b/sys/kern/kern_thread.c @@ -503,7 +503,6 @@ threadinit(void) { u_long i; lwpid_t tid0; - uint32_t flags; /* * Place an upper limit on threads which can be allocated. @@ -531,20 +530,9 @@ threadinit(void) if (tid0 != THREAD0_TID) panic("tid0 %d != %d\n", tid0, THREAD0_TID); - flags = UMA_ZONE_NOFREE; -#ifdef __aarch64__ - /* - * Force thread structures to be allocated from the direct map. - * Otherwise, superpage promotions and demotions may temporarily - * invalidate thread structure mappings. For most dynamically allocated - * structures this is not a problem, but translation faults cannot be - * handled without accessing curthread. - */ - flags |= UMA_ZONE_CONTIG; -#endif thread_zone = uma_zcreate("THREAD", sched_sizeof_thread(), thread_ctor, thread_dtor, thread_init, thread_fini, - 32 - 1, flags); + 32 - 1, UMA_ZONE_NOFREE); tidhashtbl = hashinit(maxproc / 2, M_TIDHASH, &tidhash); tidhashlock = (tidhash + 1) / 64; if (tidhashlock > 0)