From a8f0655887cc86db9d65fd5fbaf99d62424eb9b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 10 Jun 2024 07:32:30 +0100 Subject: [PATCH 1/8] KVM: arm64: Fix clobbered ELR in sync abort/SError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the hypervisor receives a SError or synchronous exception (EL2h) while running with the __kvm_hyp_vector and if ELR_EL2 doesn't point to an extable entry, it panics indirectly by overwriting ELR with the address of a panic handler in order for the asm routine it returns to to ERET into the handler. However, this clobbers ELR_EL2 for the handler itself. As a result, hyp_panic(), when retrieving what it believes to be the PC where the exception happened, actually ends up reading the address of the panic handler that called it! This results in an erroneous and confusing panic message where the source of any synchronous exception (e.g. BUG() or kCFI) appears to be __guest_exit_panic, making it hard to locate the actual BRK instruction. Therefore, store the original ELR_EL2 in the per-CPU kvm_hyp_ctxt and point the sysreg to a routine that first restores it to its previous value before running __guest_exit_panic. Fixes: 7db21530479f ("KVM: arm64: Restore hyp when panicking in guest context") Signed-off-by: Pierre-Clément Tosi Acked-by: Will Deacon Link: https://lore.kernel.org/r/20240610063244.2828978-2-ptosi@google.com Signed-off-by: Oliver Upton --- arch/arm64/kernel/asm-offsets.c | 1 + arch/arm64/kvm/hyp/entry.S | 8 ++++++++ arch/arm64/kvm/hyp/include/hyp/switch.h | 5 +++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index 81496083c041..27de1dddb0ab 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -128,6 +128,7 @@ int main(void) DEFINE(VCPU_FAULT_DISR, offsetof(struct kvm_vcpu, arch.fault.disr_el1)); DEFINE(VCPU_HCR_EL2, offsetof(struct kvm_vcpu, arch.hcr_el2)); DEFINE(CPU_USER_PT_REGS, offsetof(struct kvm_cpu_context, regs)); + DEFINE(CPU_ELR_EL2, offsetof(struct kvm_cpu_context, sys_regs[ELR_EL2])); DEFINE(CPU_RGSR_EL1, offsetof(struct kvm_cpu_context, sys_regs[RGSR_EL1])); DEFINE(CPU_GCR_EL1, offsetof(struct kvm_cpu_context, sys_regs[GCR_EL1])); DEFINE(CPU_APIAKEYLO_EL1, offsetof(struct kvm_cpu_context, sys_regs[APIAKEYLO_EL1])); diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S index f3aa7738b477..4433a234aa9b 100644 --- a/arch/arm64/kvm/hyp/entry.S +++ b/arch/arm64/kvm/hyp/entry.S @@ -83,6 +83,14 @@ alternative_else_nop_endif eret sb +SYM_INNER_LABEL(__guest_exit_restore_elr_and_panic, SYM_L_GLOBAL) + // x2-x29,lr: vcpu regs + // vcpu x0-x1 on the stack + + adr_this_cpu x0, kvm_hyp_ctxt, x1 + ldr x0, [x0, #CPU_ELR_EL2] + msr elr_el2, x0 + SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) // x2-x29,lr: vcpu regs // vcpu x0-x1 on the stack diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index 0c4de44534b7..1f4b87a73445 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -693,7 +693,7 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code) static inline void __kvm_unexpected_el2_exception(void) { - extern char __guest_exit_panic[]; + extern char __guest_exit_restore_elr_and_panic[]; unsigned long addr, fixup; struct kvm_exception_table_entry *entry, *end; unsigned long elr_el2 = read_sysreg(elr_el2); @@ -715,7 +715,8 @@ static inline void __kvm_unexpected_el2_exception(void) } /* Trigger a panic after restoring the hyp context. */ - write_sysreg(__guest_exit_panic, elr_el2); + this_cpu_ptr(&kvm_hyp_ctxt)->sys_regs[ELR_EL2] = elr_el2; + write_sysreg(__guest_exit_restore_elr_and_panic, elr_el2); } #endif /* __ARM64_KVM_HYP_SWITCH_H__ */ From ea9d7c83d14e332db9ae25eb2872b90a06ebc9e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 10 Jun 2024 07:32:31 +0100 Subject: [PATCH 2/8] KVM: arm64: Fix __pkvm_init_switch_pgd call ABI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the mismatch between the (incorrect) C signature, C call site, and asm implementation by aligning all three on an API passing the parameters (pgd and SP) separately, instead of as a bundled struct. Remove the now unnecessary memory accesses while the MMU is off from the asm, which simplifies the C caller (as it does not need to convert a VA struct pointer to PA) and makes the code slightly more robust by offsetting the struct fields from C and properly expressing the call to the C compiler (e.g. type checker and kCFI). Fixes: f320bc742bc2 ("KVM: arm64: Prepare the creation of s1 mappings at EL2") Signed-off-by: Pierre-Clément Tosi Acked-by: Will Deacon Link: https://lore.kernel.org/r/20240610063244.2828978-3-ptosi@google.com Signed-off-by: Oliver Upton --- arch/arm64/include/asm/kvm_hyp.h | 4 ++-- arch/arm64/kvm/hyp/nvhe/hyp-init.S | 24 +++++++++++++----------- arch/arm64/kvm/hyp/nvhe/setup.c | 4 ++-- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h index b05bceca3385..c838309e4ec4 100644 --- a/arch/arm64/include/asm/kvm_hyp.h +++ b/arch/arm64/include/asm/kvm_hyp.h @@ -124,8 +124,8 @@ void __noreturn __hyp_do_panic(struct kvm_cpu_context *host_ctxt, u64 spsr, #endif #ifdef __KVM_NVHE_HYPERVISOR__ -void __pkvm_init_switch_pgd(phys_addr_t phys, unsigned long size, - phys_addr_t pgd, void *sp, void *cont_fn); +void __pkvm_init_switch_pgd(phys_addr_t pgd, unsigned long sp, + void (*fn)(void)); int __pkvm_init(phys_addr_t phys, unsigned long size, unsigned long nr_cpus, unsigned long *per_cpu_base, u32 hyp_va_bits); void __noreturn __host_enter(struct kvm_cpu_context *host_ctxt); diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S index 2994878d68ea..3a2836a52e85 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S @@ -265,33 +265,35 @@ alternative_else_nop_endif SYM_CODE_END(__kvm_handle_stub_hvc) +/* + * void __pkvm_init_switch_pgd(phys_addr_t pgd, unsigned long sp, + * void (*fn)(void)); + */ SYM_FUNC_START(__pkvm_init_switch_pgd) /* Turn the MMU off */ pre_disable_mmu_workaround - mrs x2, sctlr_el2 - bic x3, x2, #SCTLR_ELx_M - msr sctlr_el2, x3 + mrs x3, sctlr_el2 + bic x4, x3, #SCTLR_ELx_M + msr sctlr_el2, x4 isb tlbi alle2 /* Install the new pgtables */ - ldr x3, [x0, #NVHE_INIT_PGD_PA] - phys_to_ttbr x4, x3 + phys_to_ttbr x5, x0 alternative_if ARM64_HAS_CNP - orr x4, x4, #TTBR_CNP_BIT + orr x5, x5, #TTBR_CNP_BIT alternative_else_nop_endif - msr ttbr0_el2, x4 + msr ttbr0_el2, x5 /* Set the new stack pointer */ - ldr x0, [x0, #NVHE_INIT_STACK_HYP_VA] - mov sp, x0 + mov sp, x1 /* And turn the MMU back on! */ dsb nsh isb - set_sctlr_el2 x2 - ret x1 + set_sctlr_el2 x3 + ret x2 SYM_FUNC_END(__pkvm_init_switch_pgd) .popsection diff --git a/arch/arm64/kvm/hyp/nvhe/setup.c b/arch/arm64/kvm/hyp/nvhe/setup.c index f4350ba07b0b..174007f3fadd 100644 --- a/arch/arm64/kvm/hyp/nvhe/setup.c +++ b/arch/arm64/kvm/hyp/nvhe/setup.c @@ -339,7 +339,7 @@ int __pkvm_init(phys_addr_t phys, unsigned long size, unsigned long nr_cpus, { struct kvm_nvhe_init_params *params; void *virt = hyp_phys_to_virt(phys); - void (*fn)(phys_addr_t params_pa, void *finalize_fn_va); + typeof(__pkvm_init_switch_pgd) *fn; int ret; BUG_ON(kvm_check_pvm_sysreg_table()); @@ -363,7 +363,7 @@ int __pkvm_init(phys_addr_t phys, unsigned long size, unsigned long nr_cpus, /* Jump in the idmap page to switch to the new page-tables */ params = this_cpu_ptr(&kvm_init_params); fn = (typeof(fn))__hyp_pa(__pkvm_init_switch_pgd); - fn(__hyp_pa(params), __pkvm_init_finalise); + fn(params->pgd_pa, params->stack_hyp_va, __pkvm_init_finalise); unreachable(); } From 6e3b773ed6bc5e783fa314b75071f022324f94a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 10 Jun 2024 07:32:32 +0100 Subject: [PATCH 3/8] KVM: arm64: nVHE: Simplify invalid_host_el2_vect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The invalid_host_el2_vect macro is used by EL2{t,h} handlers in nVHE *host* context, which should never run with a guest context loaded. Therefore, remove the superfluous vCPU context check and branch unconditionally to hyp_panic. Signed-off-by: Pierre-Clément Tosi Acked-by: Will Deacon Link: https://lore.kernel.org/r/20240610063244.2828978-4-ptosi@google.com Signed-off-by: Oliver Upton --- arch/arm64/kvm/hyp/nvhe/host.S | 6 ------ 1 file changed, 6 deletions(-) diff --git a/arch/arm64/kvm/hyp/nvhe/host.S b/arch/arm64/kvm/hyp/nvhe/host.S index 135cfb294ee5..3d610fc51f4d 100644 --- a/arch/arm64/kvm/hyp/nvhe/host.S +++ b/arch/arm64/kvm/hyp/nvhe/host.S @@ -197,12 +197,6 @@ SYM_FUNC_END(__host_hvc) sub x0, sp, x0 // x0'' = sp' - x0' = (sp + x0) - sp = x0 sub sp, sp, x0 // sp'' = sp' - x0 = (sp + x0) - x0 = sp - /* If a guest is loaded, panic out of it. */ - stp x0, x1, [sp, #-16]! - get_loaded_vcpu x0, x1 - cbnz x0, __guest_exit_panic - add sp, sp, #16 - /* * The panic may not be clean if the exception is taken before the host * context has been saved by __host_exit or after the hyp context has From 4ab3f9dd561b428460038a9bb041e92db6197f18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 10 Jun 2024 07:32:33 +0100 Subject: [PATCH 4/8] KVM: arm64: nVHE: gen-hyprel: Skip R_AARCH64_ABS32 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ignore R_AARCH64_ABS32 relocations, instead of panicking, when emitting the relocation table of the hypervisor. The toolchain might produce them when generating function calls with kCFI to represent the 32-bit type ID which can then be resolved across compilation units at link time. These are NOT actual 32-bit addresses and are therefore not needed in the final (runtime) relocation table (which is unlikely to use 32-bit absolute addresses for arm64 anyway). Signed-off-by: Pierre-Clément Tosi Acked-by: Will Deacon Link: https://lore.kernel.org/r/20240610063244.2828978-5-ptosi@google.com Signed-off-by: Oliver Upton --- arch/arm64/kvm/hyp/nvhe/gen-hyprel.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c b/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c index 6bc88a756cb7..b63f4e1c1033 100644 --- a/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c +++ b/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c @@ -50,6 +50,9 @@ #ifndef R_AARCH64_ABS64 #define R_AARCH64_ABS64 257 #endif +#ifndef R_AARCH64_ABS32 +#define R_AARCH64_ABS32 258 +#endif #ifndef R_AARCH64_PREL64 #define R_AARCH64_PREL64 260 #endif @@ -383,6 +386,9 @@ static void emit_rela_section(Elf64_Shdr *sh_rela) case R_AARCH64_ABS64: emit_rela_abs64(rela, sh_orig_name); break; + /* Allow 32-bit absolute relocation, for kCFI type hashes. */ + case R_AARCH64_ABS32: + break; /* Allow position-relative data relocations. */ case R_AARCH64_PREL64: case R_AARCH64_PREL32: From 3c6eb64876937e38672fba63f11634b9ef3013e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 10 Jun 2024 07:32:34 +0100 Subject: [PATCH 5/8] KVM: arm64: VHE: Mark __hyp_call_panic __noreturn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Given that the sole purpose of __hyp_call_panic() is to call panic(), a __noreturn function, give it the __noreturn attribute, removing the need for its caller to use unreachable(). Signed-off-by: Pierre-Clément Tosi Acked-by: Will Deacon Link: https://lore.kernel.org/r/20240610063244.2828978-6-ptosi@google.com Signed-off-by: Oliver Upton --- arch/arm64/kvm/hyp/vhe/switch.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c index 8fbb6a2e0559..ee1e1c5847e7 100644 --- a/arch/arm64/kvm/hyp/vhe/switch.c +++ b/arch/arm64/kvm/hyp/vhe/switch.c @@ -388,7 +388,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu) return ret; } -static void __hyp_call_panic(u64 spsr, u64 elr, u64 par) +static void __noreturn __hyp_call_panic(u64 spsr, u64 elr, u64 par) { struct kvm_cpu_context *host_ctxt; struct kvm_vcpu *vcpu; @@ -413,7 +413,6 @@ void __noreturn hyp_panic(void) u64 par = read_sysreg_par(); __hyp_call_panic(spsr, elr, par); - unreachable(); } asmlinkage void kvm_unexpected_el2_exception(void) From 7a928b32f1de67760e39d22d00fef99dca69fbd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 10 Jun 2024 07:32:35 +0100 Subject: [PATCH 6/8] arm64: Introduce esr_brk_comment, esr_is_cfi_brk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As it is already used in two places, move esr_comment() to a header for re-use, with a clearer name. Introduce esr_is_cfi_brk() to detect kCFI BRK syndromes, currently used by early_brk64() but soon to also be used by hypervisor code. Signed-off-by: Pierre-Clément Tosi Acked-by: Will Deacon Link: https://lore.kernel.org/r/20240610063244.2828978-7-ptosi@google.com Signed-off-by: Oliver Upton --- arch/arm64/include/asm/esr.h | 11 +++++++++++ arch/arm64/kernel/debug-monitors.c | 4 +--- arch/arm64/kernel/traps.c | 8 +++----- arch/arm64/kvm/handle_exit.c | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h index 7abf09df7033..77569d207ecf 100644 --- a/arch/arm64/include/asm/esr.h +++ b/arch/arm64/include/asm/esr.h @@ -379,6 +379,11 @@ #ifndef __ASSEMBLY__ #include +static inline unsigned long esr_brk_comment(unsigned long esr) +{ + return esr & ESR_ELx_BRK64_ISS_COMMENT_MASK; +} + static inline bool esr_is_data_abort(unsigned long esr) { const unsigned long ec = ESR_ELx_EC(esr); @@ -386,6 +391,12 @@ static inline bool esr_is_data_abort(unsigned long esr) return ec == ESR_ELx_EC_DABT_LOW || ec == ESR_ELx_EC_DABT_CUR; } +static inline bool esr_is_cfi_brk(unsigned long esr) +{ + return ESR_ELx_EC(esr) == ESR_ELx_EC_BRK64 && + (esr_brk_comment(esr) & ~CFI_BRK_IMM_MASK) == CFI_BRK_IMM_BASE; +} + static inline bool esr_fsc_is_translation_fault(unsigned long esr) { /* Translation fault, level -1 */ diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c index 64f2ecbdfe5c..024a7b245056 100644 --- a/arch/arm64/kernel/debug-monitors.c +++ b/arch/arm64/kernel/debug-monitors.c @@ -312,9 +312,7 @@ static int call_break_hook(struct pt_regs *regs, unsigned long esr) * entirely not preemptible, and we can use rcu list safely here. */ list_for_each_entry_rcu(hook, list, node) { - unsigned long comment = esr & ESR_ELx_BRK64_ISS_COMMENT_MASK; - - if ((comment & ~hook->mask) == hook->imm) + if ((esr_brk_comment(esr) & ~hook->mask) == hook->imm) fn = hook->fn; } diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index 215e6d7f2df8..9e22683aa921 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -1105,8 +1105,6 @@ static struct break_hook ubsan_break_hook = { }; #endif -#define esr_comment(esr) ((esr) & ESR_ELx_BRK64_ISS_COMMENT_MASK) - /* * Initial handler for AArch64 BRK exceptions * This handler only used until debug_traps_init(). @@ -1115,15 +1113,15 @@ int __init early_brk64(unsigned long addr, unsigned long esr, struct pt_regs *regs) { #ifdef CONFIG_CFI_CLANG - if ((esr_comment(esr) & ~CFI_BRK_IMM_MASK) == CFI_BRK_IMM_BASE) + if (esr_is_cfi_brk(esr)) return cfi_handler(regs, esr) != DBG_HOOK_HANDLED; #endif #ifdef CONFIG_KASAN_SW_TAGS - if ((esr_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM) + if ((esr_brk_comment(esr) & ~KASAN_BRK_MASK) == KASAN_BRK_IMM) return kasan_handler(regs, esr) != DBG_HOOK_HANDLED; #endif #ifdef CONFIG_UBSAN_TRAP - if ((esr_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM) + if ((esr_brk_comment(esr) & ~UBSAN_BRK_MASK) == UBSAN_BRK_IMM) return ubsan_handler(regs, esr) != DBG_HOOK_HANDLED; #endif return bug_handler(regs, esr) != DBG_HOOK_HANDLED; diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index b037f0a0e27e..d41447193e13 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -423,7 +423,7 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr, if (mode != PSR_MODE_EL2t && mode != PSR_MODE_EL2h) { kvm_err("Invalid host exception to nVHE hyp!\n"); } else if (ESR_ELx_EC(esr) == ESR_ELx_EC_BRK64 && - (esr & ESR_ELx_BRK64_ISS_COMMENT_MASK) == BUG_BRK_IMM) { + esr_brk_comment(esr) == BUG_BRK_IMM) { const char *file = NULL; unsigned int line = 0; From 8f3873a39529101213fa1109d499239d57185551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 10 Jun 2024 07:32:36 +0100 Subject: [PATCH 7/8] KVM: arm64: Introduce print_nvhe_hyp_panic helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a helper to display a panic banner soon to also be used for kCFI failures, to ensure that we remain consistent. Signed-off-by: Pierre-Clément Tosi Acked-by: Will Deacon Link: https://lore.kernel.org/r/20240610063244.2828978-8-ptosi@google.com Signed-off-by: Oliver Upton --- arch/arm64/kvm/handle_exit.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index d41447193e13..b3d6657a259d 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -411,6 +411,12 @@ void handle_exit_early(struct kvm_vcpu *vcpu, int exception_index) kvm_handle_guest_serror(vcpu, kvm_vcpu_get_esr(vcpu)); } +static void print_nvhe_hyp_panic(const char *name, u64 panic_addr) +{ + kvm_err("nVHE hyp %s at: [<%016llx>] %pB!\n", name, panic_addr, + (void *)(panic_addr + kaslr_offset())); +} + void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr, u64 elr_virt, u64 elr_phys, u64 par, uintptr_t vcpu, @@ -439,11 +445,9 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr, if (file) kvm_err("nVHE hyp BUG at: %s:%u!\n", file, line); else - kvm_err("nVHE hyp BUG at: [<%016llx>] %pB!\n", panic_addr, - (void *)(panic_addr + kaslr_offset())); + print_nvhe_hyp_panic("BUG", panic_addr); } else { - kvm_err("nVHE hyp panic at: [<%016llx>] %pB!\n", panic_addr, - (void *)(panic_addr + kaslr_offset())); + print_nvhe_hyp_panic("panic", panic_addr); } /* Dump the nVHE hypervisor backtrace */ From eca4ba5b6dff9b6ec03c9607ac297076f037fcfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Mon, 10 Jun 2024 07:32:37 +0100 Subject: [PATCH 8/8] KVM: arm64: nVHE: Support CONFIG_CFI_CLANG at EL2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compiler implements kCFI by adding type information (u32) above every function that might be indirectly called and, whenever a function pointer is called, injects a read-and-compare of that u32 against the value corresponding to the expected type. In case of a mismatch, a BRK instruction gets executed. When the hypervisor triggers such an exception in nVHE, it panics and triggers and exception return to EL1. Therefore, teach nvhe_hyp_panic_handler() to detect kCFI errors from the ESR and report them. If necessary, remind the user that EL2 kCFI is not affected by CONFIG_CFI_PERMISSIVE. Pass $(CC_FLAGS_CFI) to the compiler when building the nVHE hyp code. Use SYM_TYPED_FUNC_START() for __pkvm_init_switch_pgd, as nVHE can't call it directly and must use a PA function pointer from C (because it is part of the idmap page), which would trigger a kCFI failure if the type ID wasn't present. Signed-off-by: Pierre-Clément Tosi Acked-by: Will Deacon Link: https://lore.kernel.org/r/20240610063244.2828978-9-ptosi@google.com Signed-off-by: Oliver Upton --- arch/arm64/kvm/handle_exit.c | 10 ++++++++++ arch/arm64/kvm/hyp/nvhe/Makefile | 6 +++--- arch/arm64/kvm/hyp/nvhe/hyp-init.S | 6 +++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index b3d6657a259d..69b08ac7322d 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -417,6 +417,14 @@ static void print_nvhe_hyp_panic(const char *name, u64 panic_addr) (void *)(panic_addr + kaslr_offset())); } +static void kvm_nvhe_report_cfi_failure(u64 panic_addr) +{ + print_nvhe_hyp_panic("CFI failure", panic_addr); + + if (IS_ENABLED(CONFIG_CFI_PERMISSIVE)) + kvm_err(" (CONFIG_CFI_PERMISSIVE ignored for hyp failures)\n"); +} + void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr, u64 elr_virt, u64 elr_phys, u64 par, uintptr_t vcpu, @@ -446,6 +454,8 @@ void __noreturn __cold nvhe_hyp_panic_handler(u64 esr, u64 spsr, kvm_err("nVHE hyp BUG at: %s:%u!\n", file, line); else print_nvhe_hyp_panic("BUG", panic_addr); + } else if (IS_ENABLED(CONFIG_CFI_CLANG) && esr_is_cfi_brk(esr)) { + kvm_nvhe_report_cfi_failure(panic_addr); } else { print_nvhe_hyp_panic("panic", panic_addr); } diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile index 50fa0ffb6b7e..782b34b004be 100644 --- a/arch/arm64/kvm/hyp/nvhe/Makefile +++ b/arch/arm64/kvm/hyp/nvhe/Makefile @@ -89,9 +89,9 @@ quiet_cmd_hyprel = HYPREL $@ quiet_cmd_hypcopy = HYPCOPY $@ cmd_hypcopy = $(OBJCOPY) --prefix-symbols=__kvm_nvhe_ $< $@ -# Remove ftrace, Shadow Call Stack, and CFI CFLAGS. -# This is equivalent to the 'notrace', '__noscs', and '__nocfi' annotations. -KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS) $(CC_FLAGS_CFI), $(KBUILD_CFLAGS)) +# Remove ftrace and Shadow Call Stack CFLAGS. +# This is equivalent to the 'notrace' and '__noscs' annotations. +KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS), $(KBUILD_CFLAGS)) # Starting from 13.0.0 llvm emits SHT_REL section '.llvm.call-graph-profile' # when profile optimization is applied. gen-hyprel does not support SHT_REL and # causes a build failure. Remove profile optimization flags. diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S index 3a2836a52e85..07120b37da35 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S @@ -5,6 +5,7 @@ */ #include +#include #include #include @@ -268,8 +269,11 @@ SYM_CODE_END(__kvm_handle_stub_hvc) /* * void __pkvm_init_switch_pgd(phys_addr_t pgd, unsigned long sp, * void (*fn)(void)); + * + * SYM_TYPED_FUNC_START() allows C to call this ID-mapped function indirectly + * using a physical pointer without triggering a kCFI failure. */ -SYM_FUNC_START(__pkvm_init_switch_pgd) +SYM_TYPED_FUNC_START(__pkvm_init_switch_pgd) /* Turn the MMU off */ pre_disable_mmu_workaround mrs x3, sctlr_el2