Re: [PATCH v2 5/8] KVM: arm64: Handle VNCR TLB invalidation race with vcpu_put() VNCR unmapping

On Thu, Aug 06, 2026 at 10:10:23AM +0800, Marc Zyngier wrote:
> While VNCR TLB invalidation always occurs under the MMU lock,
> vcpu_put() doesn't, while it unmaps the VNCR page.
>
> The problem is that the invalidation evaluates vncr_tlb::cpu to
> decide whether an unmapping needs to take place (cpu != -1) before
> performing it. On the other hand, this_cpu_reset_vncr_fixmap()
> unconditionally unmaps if L1_VNCR_MAPPED is set.
>
> These two obviously can race, with a TOCTOU pattern on the TLBI
> path, and a BUG_ON() on the vcpu_put() path. And the two can end-up
> calling vncr_fixmap(-1), with extra lethal effects.

...

>  static void this_cpu_reset_vncr_fixmap(struct kvm_vcpu *vcpu)
>  {
>  	if (!host_data_test_flag(L1_VNCR_MAPPED))
>  		return;
>
> -	BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id());
>  	BUG_ON(is_hyp_ctxt(vcpu));
>
> -	clear_fixmap(vncr_fixmap(vcpu->arch.vncr_tlb->cpu));
> -	vcpu->arch.vncr_tlb->cpu = -1;
> +	/*
> +	 * Unconditionally unmap the local VNCR if we have lost the race
> +	 * against a concurrent TLBI. Otherwise we could end-up running
> +	 * another vcpu with VNCR still mapped if the TLBI thread is
> +	 * preempted between the exchange and the clear_fixmap().

NICE CATCH.

I thought about the remain changed code path in this patch
and they have MMU lock w/ preemption disabled. This is one
trick racing.

Reviewed-by: Yuan Yao <[email protected]>

> +	 *
> +	 * Note that we do not care about the TLBI nuking the fixmap behind
> +	 * the back of an running vcpu. This will only generate a fault and
> +	 * possibly a retranslation.
> +	 */
> +	if (unmap_l1_vncr(vcpu->arch.vncr_tlb) == -1)
> +		clear_fixmap(vncr_fixmap(smp_processor_id()));
>  	host_data_clear_flag(L1_VNCR_MAPPED);
>  }
>
> @@ -995,8 +1019,7 @@ u16 get_asid_by_regime(struct kvm_vcpu *vcpu, enum trans_regime regime)
>  static void invalidate_vncr(struct vncr_tlb *vt)
>  {
>  	vt->valid = false;
> -	if (vt->cpu != -1)
> -		clear_fixmap(vncr_fixmap(vt->cpu));
> +	unmap_l1_vncr(vt);
>  }
>
>  static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,
> @@ -1452,7 +1475,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
>  		vt->hpa = pfn << PAGE_SHIFT;
>  		vt->hpa_writable = writable;
>  		vt->valid = true;
> -		vt->cpu = -1;
> +		atomic_set(&vt->cpu, -1);
>
>  		kvm_make_request(KVM_REQ_MAP_L1_VNCR_EL2, vcpu);
>  		kvm_release_faultin_page(vcpu->kvm, page, false, vt->wr.pw && vt->hpa_writable);
> @@ -1583,8 +1606,6 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
>  	if (vt->wr.nG && get_asid_by_regime(vcpu, TR_EL20) != vt->wr.asid)
>  		return;
>
> -	vt->cpu = smp_processor_id();
> -
>  	if (vt->hpa_writable && vt->wr.pw && vt->wr.pr)
>  		prot = PAGE_KERNEL;
>  	else if (vt->wr.pr)
> @@ -1599,7 +1620,8 @@ static void kvm_map_l1_vncr(struct kvm_vcpu *vcpu)
>  	 * FIXME: WO doesn't work at all, need POE support in the kernel.
>  	 */
>  	if (pgprot_val(prot) != pgprot_val(PAGE_NONE)) {
> -		__set_fixmap(vncr_fixmap(vt->cpu), vt->hpa, prot);
> +		atomic_set(&vt->cpu, smp_processor_id());
> +		__set_fixmap(vncr_fixmap(atomic_read(&vt->cpu)), vt->hpa, prot);
>  		host_data_set_flag(L1_VNCR_MAPPED);
>  	}
>  }
> --
> 2.47.3