Re: [PATCH 3/3] KVM: arm64: Drop redundant READ_ONCE() in pkvm_hyp_vm_is_created()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Teardown sequence inversion in `kvm_arch_destroy_vm()` causes memory leak of protected guest pages.
--

commit 3c4352fb45fd315836f7f7aa2ad63af580a902d7
Author: Fuad Tabba <tabba@google.com>

KVM: arm64: Drop redundant READ_ONCE() in pkvm_hyp_vm_is_created()

This patch removes an unnecessary READ_ONCE() around the read of
kvm->arch.pkvm.is_created. The read is already serialised against concurrent
writers by either config_lock, slots_lock, or the teardown VM lifecycle.

> diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
> index 67b90a58fbea2..008766273912e 100644
> --- a/arch/arm64/kvm/pkvm.c
> +++ b/arch/arm64/kvm/pkvm.c
> @@ -185,7 +185,11 @@ static int __pkvm_create_hyp_vm(struct kvm *kvm)
>  
>  bool pkvm_hyp_vm_is_created(struct kvm *kvm)
>  {
> -	return READ_ONCE(kvm->arch.pkvm.is_created);
> +	/*
> +	 * Serialised by config_lock/slots_lock, or by VM lifecycle at
> +	 * teardown, so a plain read suffices.
> +	 */

[Severity: High]
This isn't a bug introduced by this patch, but since the comment mentions the
teardown lifecycle, is there a teardown sequence inversion in
kvm_arch_destroy_vm() that leaks protected guest pages?

Looking at kvm_arch_destroy_vm() in arch/arm64/kvm/arm.c, it calls
pkvm_destroy_hyp_vm() before kvm_uninit_stage2_mmu().

Because kvm_uninit_stage2_mmu() hasn't run yet, __pkvm_start_teardown_vm()
is not invoked. This causes the __pkvm_finalize_teardown_vm hypercall inside
pkvm_destroy_hyp_vm() to be rejected by the hypervisor, and the host clears
the handle via __pkvm_destroy_hyp_vm():

arch/arm64/kvm/pkvm.c:__pkvm_destroy_hyp_vm() {
    ...
    kvm->arch.pkvm.handle = 0;
}

Subsequently, when kvm_uninit_stage2_mmu() does run, it eventually calls
pkvm_pgtable_stage2_destroy_range(), which returns early due to the cleared
handle:

arch/arm64/kvm/pkvm.c:pkvm_pgtable_stage2_destroy_range() {
    if (!handle)
        return;
    ...
}

Does this sequence skip page reclamation and unsharing, permanently leaking
all memory donated to the protected guest?

> +	return kvm->arch.pkvm.is_created;
>  }
>  
>  int pkvm_create_hyp_vm(struct kvm *kvm)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618090128.3913688-1-tabba@google.com?part=3