Re: [PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again

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

Pre-existing issues:
- [Critical] Cross-CPU invalidation of VNCR TLBs (e.g., via MMU notifiers) clears the fixmap and resets `vt->cpu` but fails to clear the per-CPU `L1_VNCR_MAPPED` flag on the CPU where the vCPU is running. When the vCPU is subsequently scheduled out, `this_cpu_reset_vncr_fixmap()` checks the local flag, finds it set, and hits a `BUG_ON` because `vt->cpu` is `-1`. This allows unprivileged host userspace to panic the host kernel.
- [High] The fast-path check in `kvm_hyp_handle_tlbi_el2()` skips VNCR TLB invalidation for EL2 TLBI instructions when `E2H` or `TGE` is 0, leaving stale translations in the software VNCR TLB.
- [High] A TOCTOU race exists between lockless guest page table walks and guest TLBI intercepts, allowing stale VNCR translations to survive invalidation.
--

--- Patch [6]: [PATCH 6/6] KVM: arm64: Add VNCR TLB tracking again ---
commit bde680578e8485472440ba4858d93cb92173897d
Author: Marc Zyngier <[email protected]>
Date:   Sat Aug 1 12:48:18 2026 +0000

    KVM: arm64: Add VNCR TLB tracking again
    
    This patch re-introduces VNCR TLB tracking to force EL2 S1 TLBI handling
    onto the slow path when valid VNCR TLBs exist. It uses a VM-wide atomic
    counter to track the presence of valid VNCR TLBs, updating it under the
    MMU lock.

> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index c09b1d411c584..eb59549ec2172 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -424,10 +424,11 @@ static bool kvm_hyp_handle_tlbi_el2(struct kvm_vcpu *vcpu, u64 *exit_code)
>  		return false;
>  
>  	/*
> -	 * If we have to check for any VNCR mapping being invalidated,
> +	 * If we have to check for any VNCR TLB being invalidated,
>  	 * go back to the slow path for further processing.
>  	 */
> -	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu))
> +	if (vcpu_el2_e2h_is_set(vcpu) && vcpu_el2_tge_is_set(vcpu) &&
> +	    atomic_read(&vcpu->kvm->arch.vncr_tlb_count))
>  		return false;

[Severity: High]
This isn't a bug introduced by this patch, but does this check skip VNCR TLB
invalidation for EL2 TLBI instructions when the guest is in nVHE mode?

If a guest running with E2H=0 changes its EL2 page tables for the VNCR_EL2
mapping and issues an EL2 TLBI, this condition evaluates to false.

The fast-path will return true, skipping the slow path that would normally
handle the invalidation. Could this leave stale translations in the software
VNCR TLB?

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 8d5b0208ad728..2a7dd9f9fb597 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
[ ... ]
> @@ -998,9 +999,11 @@ u16 get_asid_by_regime(struct kvm_vcpu *vcpu, enum trans_regime regime)
>  	return asid;
>  }
>  
> -static void invalidate_vncr(struct vncr_tlb *vt)
> +static void invalidate_vncr(struct kvm *kvm, struct vncr_tlb *vt)
>  {
> +	BUG_ON(!vt->valid);
>  	vt->valid = false;
> +	atomic_dec(&kvm->arch.vncr_tlb_count);
>  	if (vt->cpu != -1)
>  		unmap_l1_vncr(vt);
>  }

[Severity: Critical]
This is a pre-existing issue, but does this cross-CPU invalidation path leave
the per-CPU L1_VNCR_MAPPED flag set locally?

If an MMU notifier on CPU B calls kvm_invalidate_vncr_ipa() which reaches
invalidate_vncr(), unmap_l1_vncr() will clear the fixmap and set vt->cpu = -1.

When CPU A (which was running the vCPU) exits the guest and calls
this_cpu_reset_vncr_fixmap(), won't it see its local L1_VNCR_MAPPED flag still
set and trigger BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id()) because
vt->cpu is now -1?

[ ... ]
> @@ -1460,6 +1471,8 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
>  		vt->valid = true;
>  		vt->cpu = -1;
>  
> +		atomic_inc(&vcpu->kvm->arch.vncr_tlb_count);
> +
>  		kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
>  		kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
>  	}

[Severity: High]
This is a pre-existing issue, but is there a time-of-check to time-of-use race
between the lockless guest page table walk and guest TLBI intercepts here?

CPU1 locklessly resolves the guest virtual address via __kvm_translate_va()
earlier in kvm_translate_vncr().

Before CPU1 acquires the mmu_lock and increments vncr_tlb_count, CPU2 could
modify the guest page table and issue a TLBI.

The TLBI on CPU2 would see vncr_tlb_count == 0 (or vt->valid == false) and
complete. CPU1 would then acquire mmu_lock, pass the mmu_invalidate_retry()
check (which only tracks host MMU notifiers), and install the stale
translation.

Could this result in caching a stale VNCR mapping?

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