Re: [PATCH] KVM: arm64: Drop %pB on nVHE panic when stage-2 is active

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch fixes a stage-2 fault deadlock in `print_nvhe_hyp_panic()` by removing the `%pB` modifier, but misses `kvm_nvhe_dump_backtrace_entry()` and `dump_kernel_instr()` which both access lazily-mapped host memory later in the exact same panic path, causing the same deadlock.
--

--- Patch [1]: [PATCH] KVM: arm64: Drop %pB on nVHE panic when stage-2 is active ---
commit cbd94a7b340a7dc4be01336a040cb8c3d9690ab4
Author: Vincent Donnefort <[email protected]>

KVM: arm64: Drop %pB on nVHE panic when stage-2 is active

This commit removes the %pB format specifier from print_nvhe_hyp_panic()
when running in protected KVM mode without stage-2 panic disabling. This
prevents a deadlock caused by a stage-2 fault when accessing unmapped
kallsyms memory while the host stage-2 lock is held.

> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 54aedf93c78b6..2e1951c215ae8 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -509,8 +509,12 @@ void handle_exit_early(struct kvm_vcpu *vcpu, int exception_index)
>  
>  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()));
> +	/* Kallsyms might not be mapped in the host stage-2 */
> +	if (is_protected_kvm_enabled() && !IS_ENABLED(CONFIG_PKVM_DISABLE_STAGE2_ON_PANIC))
> +		kvm_err("nVHE hyp %s at: %016llx!\n", name, panic_addr);
> +	else
> +		kvm_err("nVHE hyp %s at: [<%016llx>] %pB!\n", name, panic_addr,
> +			(void *)(panic_addr + kaslr_offset()));
>  }

[Severity: High]
This is a pre-existing issue, but does this same deadlock still trigger
later in the panic path?

While this fixes the memory access in print_nvhe_hyp_panic(), the handler
nvhe_hyp_panic_handler() subsequently calls kvm_nvhe_dump_backtrace() and
dump_kernel_instr().

In arch/arm64/kvm/stacktrace.c:kvm_nvhe_dump_backtrace_entry(), there is
another use of %pB:

	kvm_err(" [<%016lx>] %pB\n", where, (void *)(where + kaslr_offset()));

Additionally, nvhe_hyp_panic_handler() calls dump_kernel_instr() which also
reads the faulting instruction directly from memory in
arch/arm64/kernel/traps.c:

	bad = aarch64_insn_read(&((u32 *)kaddr)[i], &val);

Will both of these also access lazily-mapped host stage-2 memory, trigger
a stage-2 data abort, and deadlock when trying to re-acquire host_mmu.lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1