Sashiko reports that there is a race between initialising vncr_tlb
and making use of it, as we don't hold the mmu_lock at this point.
Additionally, it identifies a memory leak, should userspace repeatedly
invokes the KVM_RUN ioctl after a failure of kvm_arch_vcpu_run_pid_change(),
as we assign vncr_tlb blindly on first run, irrespective of prior
allocations.
Slap the two bugs in one go by taking the kvm->mmu_lock on assigning
vncr_tlb, preventing the race for good, and by checking that vncr_tlb
is indeed NULL prior to allocation.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260607180815.85FBC1F00893@smtp.kernel.org
Reviewed-by: Oliver Upton <oupton@kernel.org>
Link: https://patch.msgid.link/20260608081108.2244133-1-maz@kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR4_EL1, NV_frac, NV2_ONLY))
return 0;
- vcpu->arch.vncr_tlb = kzalloc_obj(*vcpu->arch.vncr_tlb,
- GFP_KERNEL_ACCOUNT);
+ if (!vcpu->arch.vncr_tlb) {
+ struct vncr_tlb *vt = kzalloc_obj(*vcpu->arch.vncr_tlb,
+ GFP_KERNEL_ACCOUNT);
+
+ /*
+ * Taking the lock on assignment ensures that the TLB is
+ * seen as initialised when following the pointer (release
+ * semantics of the unlock), and avoids having acquires on
+ * each user which already take the lock.
+ */
+ scoped_guard(write_lock, &vcpu->kvm->mmu_lock)
+ vcpu->arch.vncr_tlb = vt;
+ }
+
if (!vcpu->arch.vncr_tlb)
return -ENOMEM;