Re: [PATCH v3 1/2] KVM: arm64: nv: Fix life cycle of the nested_mmus array

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

New issues:
- [High] Lockless iteration in `kvm_arch_flush_shadow_all` races with `kvm_vcpu_init_nested`, causing a garbage/NULL pointer dereference.

Pre-existing issues:
- [High] Memory leak of nested MMU stage-2 page tables (`mmu->pgt`) and per-CPU data when VM is destroyed after `mmu_notifier_release` has been bypassed.
--

--- Patch [1]: [PATCH v3 1/2] KVM: arm64: nv: Fix life cycle of the nested_mmus array ---
commit 2f4c64e5fe4ce7a7346ea83c7e69ecaa68ee9290
Author: Marc Zyngier <[email protected]>
Date:   Fri Aug 21 16:18:28 2026 +0000

    KVM: arm64: nv: Fix life cycle of the nested_mmus array
    
This commit modifies the life cycle of the nested_mmus array by turning it 
from an array of structures into an array of pointers allocated at VM creation. 
It prevents issues where reallocation during VCPU_INIT could leave running 
vCPUs with freed pointers, ensuring S2 MMUs are incrementally allocated without 
reallocation.

Link: https://lore.kernel.org/r/[email protected]

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 8b080804bc90b..b53219e048bdf 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
[ ... ]
> @@ -337,6 +340,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
>  
>  	kvm_unshare_hyp(kvm, kvm + 1);
>  
> +	kvfree(kvm->arch.nested_mmus);

[Severity: High]
This is a pre-existing issue, but does this teardown path leak the nested MMU 
stage-2 page tables?

When a VM file descriptor is shared with a child process and the parent exits,
the parent's exit_mmap() calls mmu_notifier_release() which unhashes the
notifier. If the child process then initializes vCPUs, it allocates new nested
MMU PGDs via init_nested_s2_mmu().

When the child exits, kvm_arch_destroy_vm() is called. Because the notifier is
already unhashed, it bypasses the release callback, skipping
kvm_arch_flush_shadow_all().

Since the cleanup path fails to call kvm_free_stage2_pgd() for the MMUs before
freeing the nested_mmus array, the page tables (mmu->pgt) and per-CPU
allocations (mmu->last_vcpu_ran) appear to be permanently leaked.

>  	kvm_arm_teardown_hypercalls(kvm);
>  }
>  
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 17123f0b6daba..9070d5bc5af03 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
[ ... ]
> @@ -84,51 +89,40 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
[ ... ]
> +		guard(write_lock)(&kvm->mmu_lock);
>  
> -		for (int i = kvm->arch.nested_mmus_size; i < num_mmus; i++)
> -			kvm_free_stage2_pgd(&kvm->arch.nested_mmus[i]);
> +		for (i = 0; i < S2_MMU_PER_VCPU; i++)
> -		free_page((unsigned long)vcpu->arch.ctxt.vncr_array);
> -		vcpu->arch.ctxt.vncr_array = NULL;
> +			kvm->arch.nested_mmus[i + kvm->arch.nested_mmus_size] = &tmp[i];
>  
> -		return ret;
> +		kvm->arch.nested_mmus_size += S2_MMU_PER_VCPU;

[Severity: High]
Could this result in a data race on weakly ordered architectures?

Here in kvm_vcpu_init_nested(), nested_mmus_size is incremented immediately
after storing the new MMU pointers in the array. Since there are no memory
barriers between the pointer stores and the size increment, the CPU could
reorder the size increment to be visible before the array elements.

If kvm_arch_flush_shadow_all() concurrently iterates over the array locklessly
(for example, during process exit via mmu_notifier_release), it could read the
updated size but observe uninitialized array memory. This would result in
reading a garbage pointer, leading to a kernel panic when it dereferences it
in atomic_read(&mmu->refcnt).

>  	}
>  
> -	kvm->arch.nested_mmus_size = num_mmus;
> -
>  	return 0;
>  }

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