Re: [PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] By modifying `invalidate_vncr()` to call the new `unmap_l1_vncr()` helper, this patch asynchronously sets `vt->cpu = -1` when invalidating a remote vCPU's VNCR TLB. However, the host CPU's `L1_VNCR_MAPPED` flag remains set on the remote CPU. When the remote vCPU is eventually scheduled out and executes the lockless `this_cpu_reset_vncr_fixmap()`, it observes `L1_VNCR_MAPPED` is true but finds `vt->cpu` has been changed to `-1`. This triggers a deterministic host kernel panic via `BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id())`. A guest can trivially exploit this by issuing a TLBI from one vCPU while another is running.

Pre-existing issues:
- [Critical] In `kvm_map_l1_vncr()`, `vt->cpu` is unconditionally set to `smp_processor_id()`. If the mapping has no read permissions (`prot == PAGE_NONE`), the function returns without mapping the fixmap or setting the `L1_VNCR_MAPPED` flag. When the vCPU is scheduled out, `this_cpu_reset_vncr_fixmap()` checks `L1_VNCR_MAPPED` and returns early, failing to reset `vt->cpu` to `-1`. This leaves a stale CPU ID in `vt->cpu`. If the host later invalidates this vCPU's VNCR TLB, `invalidate_vncr()` reads the stale CPU ID and maliciously clears the fixmap for that CPU. If another vCPU is running on that CPU and using the fixmap slot, its VNCR mapping is unexpectedly destroyed, leading to a host Data Abort kernel panic when accessed. This problem wasn't introduced by this patch, but remains a critical vulnerability.
- [Critical] A Time-of-Check to Time-of-Use (TOCTOU) race exists on `vt->cpu` between `invalidate_vncr()` and the lockless `this_cpu_reset_vncr_fixmap()`. `invalidate_vncr()` executes `if (vt->cpu != -1)` and then calls `unmap_l1_vncr()`, which evaluates `vncr_fixmap(vt->cpu)`. Because `this_cpu_reset_vncr_fixmap()` is called from `kvm_arch_vcpu_put` without holding `kvm->mmu_lock`, it can run concurrently on the target vCPU's host CPU and set `vt->cpu = -1` exactly between the check and the use. `unmap_l1_vncr()` then evaluates `vncr_fixmap(-1)`, triggering `BUG_ON(__c >= NR_CPUS)` and panicking the host. This problem wasn't introduced by this patch, but remains a critical vulnerability.
--

--- Patch [5]: [PATCH 5/6] KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation ---
commit 9f716e0407bd9a5d530b0e1d1e94c5434eaeedb6
Author: Marc Zyngier <maz@kernel.org>

KVM: arm64: Couple VNCR fixmap clearing and CPU number invalidation

A mapped VNCR translation must indicate which CPU it is resident.
Unmapping such a translation must make this CPU number -1. Couple the unmap
and the setting to -1 in a helper, so that we are always sure they are done
together.

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 69a4652f39336..8d5b0208ad728 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c

[ ... ]

> @@ -895,6 +895,12 @@ void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu)
>  	}
>  }
>  
> +static void unmap_l1_vncr(struct vncr_tlb *vt)
> +{
> +	clear_fixmap(vncr_fixmap(vt->cpu));
> +	vt->cpu = -1;
> +}
> +
>  static void this_cpu_reset_vncr_fixmap(struct kvm_vcpu *vcpu)
>  {
>  	if (!host_data_test_flag(L1_VNCR_MAPPED))
>  		return;

[Severity: Critical]
This is a pre-existing issue, but can a stale CPU ID be left in vt->cpu if 
we return early here?

In kvm_map_l1_vncr(), vt->cpu is unconditionally set to smp_processor_id().
If the mapping has no read permissions (prot == PAGE_NONE), the function
returns without mapping the fixmap or setting the L1_VNCR_MAPPED flag.

When the vCPU is later scheduled out, this_cpu_reset_vncr_fixmap() returns
early at this check because L1_VNCR_MAPPED is unset, failing to reset
vt->cpu to -1.

If the host later invalidates this vCPU's VNCR TLB, invalidate_vncr() reads
the stale CPU ID and maliciously clears the fixmap for that CPU. If another
vCPU is running on that CPU and using the fixmap slot, its VNCR mapping is
unexpectedly destroyed, leading to a host Data Abort when accessed.

>  	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;
> +	unmap_l1_vncr(vcpu->arch.vncr_tlb);
>  	host_data_clear_flag(L1_VNCR_MAPPED);
>  }
>  
> @@ -997,7 +1002,7 @@ static void invalidate_vncr(struct vncr_tlb *vt)
>  {
>  	vt->valid = false;
>  	if (vt->cpu != -1)

[Severity: Critical]
This is a pre-existing issue, but does this check have a time-of-check to
time-of-use race?

In invalidate_vncr(), we check if vt->cpu != -1 and then call
unmap_l1_vncr(vt), which evaluates vncr_fixmap(vt->cpu) internally.

Because this_cpu_reset_vncr_fixmap() is called from kvm_arch_vcpu_put()
without holding kvm->mmu_lock, it can run concurrently on the target vCPU's
host CPU and set vt->cpu = -1 exactly between the check and the use.

If that happens, unmap_l1_vncr() evaluates vncr_fixmap(-1), which triggers
BUG_ON(__c >= NR_CPUS) and panics the host.

> -		clear_fixmap(vncr_fixmap(vt->cpu));
> +		unmap_l1_vncr(vt);

[Severity: Critical]
Can this trigger a BUG_ON() when a remote vCPU is scheduled out?

By modifying invalidate_vncr() to call unmap_l1_vncr(), this path now
asynchronously sets vt->cpu = -1 when invalidating a remote vCPU's
VNCR TLB. However, the host CPU's L1_VNCR_MAPPED flag remains set on
the remote CPU.

When the remote vCPU is eventually scheduled out, it will execute the
lockless this_cpu_reset_vncr_fixmap():

    if (!host_data_test_flag(L1_VNCR_MAPPED))
        return;

    BUG_ON(vcpu->arch.vncr_tlb->cpu != smp_processor_id());

Since L1_VNCR_MAPPED is still true, the code reaches the BUG_ON(), but
vt->cpu has been changed to -1, which deterministicly panics the host
kernel. A guest could trivially trigger this by issuing a TLBI from one
vCPU while another is running.

>  }
>  
>  static bool vncr_tlb_intersects(struct vncr_tlb *vt, u64 addr,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801124818.366274-1-maz@kernel.org?part=5