[PATCH] KVM: x86/mmu: Protect noncoherent DMA zaps with SRCU

[PATCH] KVM: x86/mmu: Protect noncoherent DMA zaps with SRCU

[Chengfeng Ye]

Protect the noncoherent DMA zap with KVM's SRCU so that memslots and
their architecture-specific metadata remain alive if the rmap walk
drops mmu_lock to reschedule.

The VFIO noncoherent-DMA path invokes
kvm_arch_register_noncoherent_dma() without holding slots_lock or an
SRCU read lock. kvm_zap_gfn_range() can then enter
__walk_slot_rmaps(), which retains pointers to a memslot and its rmap
while cond_resched_rwlock_write() temporarily drops mmu_lock.

The race looks like this:

  CPU 0: VFIO coherency update          CPU 1: memslot delete
  ----------------------------          ---------------------
  kvm_vfio_set_attr()
    kvm_arch_register_noncoherent_dma()
      kvm_zap_gfn_range()
        __walk_slot_rmaps()
          iterator.rmap = slot->arch.rmap
          cond_resched_rwlock_write()
            drop mmu_lock

                                        KVM_SET_USER_MEMORY_REGION(DELETE)
                                          kvm_arch_flush_shadow_memslot()
                                            zap SPTEs under mmu_lock
                                          kvm_swap_active_memslots()
                                            synchronize_srcu_expedited()
                                          kvm_free_memslot()
                                            vfree(slot->arch.rmap[i])
                                            kfree(slot)

          reacquire mmu_lock
          slot_rmap_walk_next()
            read freed iterator.rmap

The delete path is allowed to free the old memslot because the zap path
holds no SRCU read lock. synchronize_srcu_expedited() therefore does not
wait for the rmap walk before kvm_free_memslot() releases the old slot
and its rmap array. When the zap resumes, slot_rmap_walk_next() reads
from freed memory.

KASAN reported:

  BUG: KASAN: vmalloc-out-of-bounds in
  slot_rmap_walk_next+0x82/0x1c0
  Read of size 8 at addr ffffc900005c1008

  Call Trace:
   slot_rmap_walk_next+0x82/0x1c0
   __kvm_rmap_zap_gfn_range+0x17a/0x280
   kvm_zap_gfn_range+0x2a6/0x6a0
   kvm_vfio_set_attr+0x576/0x770
   kvm_device_ioctl+0x1ff/0x3b0
   __x64_sys_ioctl+0x134/0x1c0

Hold SRCU across the zap so that memslot deletion waits for the walk to
finish before freeing the old slot, without changing zap behavior.

Fixes: 362ff6dca541 ("KVM: x86/mmu: Zap KVM TDP when noncoherent DMA assignment starts/stops")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 arch/x86/kvm/x86.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 69469bbdc84a..2114553f3159 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -14092,8 +14092,12 @@ static void kvm_noncoherent_dma_assignment_start_or_stop(struct kvm *kvm)
 	 *
 	 * If KVM always honors guest PAT, however, there is nothing to do.
 	 */
-	if (kvm_check_has_quirk(kvm, KVM_X86_QUIRK_IGNORE_GUEST_PAT))
+	if (kvm_check_has_quirk(kvm, KVM_X86_QUIRK_IGNORE_GUEST_PAT)) {
+		int idx = srcu_read_lock(&kvm->srcu);
+
 		kvm_zap_gfn_range(kvm, gpa_to_gfn(0), gpa_to_gfn(~0ULL));
+		srcu_read_unlock(&kvm->srcu, idx);
+	}
 }
 
 void kvm_arch_register_noncoherent_dma(struct kvm *kvm)
-- 
2.43.0

Re: [PATCH] KVM: x86/mmu: Protect noncoherent DMA zaps with SRCU

[Huang, Kai]

On Sun, 2026-08-23 at 03:02 +0800, Chengfeng Ye wrote:
> Protect the noncoherent DMA zap with KVM's SRCU so that memslots and
> their architecture-specific metadata remain alive if the rmap walk
> drops mmu_lock to reschedule.
> 
> The VFIO noncoherent-DMA path invokes
> kvm_arch_register_noncoherent_dma() without holding slots_lock or an
> SRCU read lock. kvm_zap_gfn_range() can then enter
> __walk_slot_rmaps(), which retains pointers to a memslot and its rmap
> while cond_resched_rwlock_write() temporarily drops mmu_lock.

kvm_zap_gfn_range() can invoke 

   slots = __kvm_memslots(kvm, i);

internally (from kvm_rmap_zap_gfn_range()).  AFAICT this itself is enough that
the caller should either grab read lock of KVM's SRCU or slots_lock.

For instance, __kvm_set_or_clear_apicv_inhibit() is already holding KVM's SRCU
read lock when calling kvm_zap_gfn_range() to zap the GFN of guest's APIC page
(which is only 1 page thus won't yield).

> 
> The race looks like this:
> 
>   CPU 0: VFIO coherency update          CPU 1: memslot delete
>   ----------------------------          ---------------------
>   kvm_vfio_set_attr()
>     kvm_arch_register_noncoherent_dma()
>       kvm_zap_gfn_range()
>         __walk_slot_rmaps()
>           iterator.rmap = slot->arch.rmap
>           cond_resched_rwlock_write()
>             drop mmu_lock
> 
>                                         KVM_SET_USER_MEMORY_REGION(DELETE)
>                                           kvm_arch_flush_shadow_memslot()
>                                             zap SPTEs under mmu_lock
>                                           kvm_swap_active_memslots()
>                                             synchronize_srcu_expedited()

Isn't kvm_swap_active_memslots() called _before_
kvm_arch_flush_shadow_memslot()?


>                                           kvm_free_memslot()
>                                             vfree(slot->arch.rmap[i])
>                                             kfree(slot)
> 
>           reacquire mmu_lock
>           slot_rmap_walk_next()
>             read freed iterator.rmap
> 
> The delete path is allowed to free the old memslot because the zap path
> holds no SRCU read lock. synchronize_srcu_expedited() therefore does not
> wait for the rmap walk before kvm_free_memslot() releases the old slot
> and its rmap array. When the zap resumes, slot_rmap_walk_next() reads
> from freed memory.
> 
> KASAN reported:
> 
>   BUG: KASAN: vmalloc-out-of-bounds in
>   slot_rmap_walk_next+0x82/0x1c0
>   Read of size 8 at addr ffffc900005c1008
> 
>   Call Trace:
>    slot_rmap_walk_next+0x82/0x1c0
>    __kvm_rmap_zap_gfn_range+0x17a/0x280
>    kvm_zap_gfn_range+0x2a6/0x6a0
>    kvm_vfio_set_attr+0x576/0x770
>    kvm_device_ioctl+0x1ff/0x3b0
>    __x64_sys_ioctl+0x134/0x1c0
> 
> Hold SRCU across the zap so that memslot deletion waits for the walk to
> finish before freeing the old slot, without changing zap behavior.
> 
> Fixes: 362ff6dca541 ("KVM: x86/mmu: Zap KVM TDP when noncoherent DMA assignment starts/stops")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>

Code change LGTM, feel free to add:

Reviewed-by: Kai Huang <kai.huang@intel.com>

Nit: the short-log uses "x86/mmu" as topic, but maybe "x86" is more appropriate
since the code change is more like at x86 level?

> ---
>  arch/x86/kvm/x86.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 69469bbdc84a..2114553f3159 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -14092,8 +14092,12 @@ static void kvm_noncoherent_dma_assignment_start_or_stop(struct kvm *kvm)
>  	 *
>  	 * If KVM always honors guest PAT, however, there is nothing to do.
>  	 */
> -	if (kvm_check_has_quirk(kvm, KVM_X86_QUIRK_IGNORE_GUEST_PAT))
> +	if (kvm_check_has_quirk(kvm, KVM_X86_QUIRK_IGNORE_GUEST_PAT)) {
> +		int idx = srcu_read_lock(&kvm->srcu);
> +
>  		kvm_zap_gfn_range(kvm, gpa_to_gfn(0), gpa_to_gfn(~0ULL));
> +		srcu_read_unlock(&kvm->srcu, idx);
> +	}
>  }

Re: [PATCH] KVM: x86/mmu: Protect noncoherent DMA zaps with SRCU

[Sean Christopherson]

On Sun, Aug 23, 2026, Chengfeng Ye wrote:
> Fixes: 362ff6dca541 ("KVM: x86/mmu: Zap KVM TDP when noncoherent DMA assignment starts/stops")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
>  arch/x86/kvm/x86.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 69469bbdc84a..2114553f3159 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -14092,8 +14092,12 @@ static void kvm_noncoherent_dma_assignment_start_or_stop(struct kvm *kvm)
>  	 *
>  	 * If KVM always honors guest PAT, however, there is nothing to do.
>  	 */
> -	if (kvm_check_has_quirk(kvm, KVM_X86_QUIRK_IGNORE_GUEST_PAT))
> +	if (kvm_check_has_quirk(kvm, KVM_X86_QUIRK_IGNORE_GUEST_PAT)) {
> +		int idx = srcu_read_lock(&kvm->srcu);

Please do:

		guard(srcu)(&kvm->srcu);

even though this is tagged for stable.

Alternatively, what if we have kvm_zap_gfn_range() acquire SRCU?  Nesting "locks"
is a-ok, and two of the three users of kvm_zap_gfn_range() have had this bug (see
commit 074c00800719 ("KVM: x86: Use SRCU to protect zap in
__kvm_set_or_clear_apicv_inhibit()").

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 6c13da942bfc..34e9eebb38af 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -7072,6 +7072,8 @@ void kvm_zap_gfn_range(struct kvm *kvm, gfn_t gfn_start, gfn_t gfn_end)
 	if (WARN_ON_ONCE(gfn_end <= gfn_start))
 		return;
 
+	guard(srcu)(&kvm->srcu);
+
 	write_lock(&kvm->mmu_lock);
 
 	kvm_mmu_invalidate_start(kvm);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0626e835e9eb..9ecbaea69677 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7875,10 +7875,8 @@ void __kvm_set_or_clear_apicv_inhibit(struct kvm *kvm,
 		kvm->arch.apicv_inhibit_reasons = new;
 		if (new) {
 			unsigned long gfn = gpa_to_gfn(APIC_DEFAULT_PHYS_BASE);
-			int idx = srcu_read_lock(&kvm->srcu);
 
 			kvm_zap_gfn_range(kvm, gfn, gfn+1);
-			srcu_read_unlock(&kvm->srcu, idx);
 		}
 	} else {
 		kvm->arch.apicv_inhibit_reasons = new;


> +
>  		kvm_zap_gfn_range(kvm, gpa_to_gfn(0), gpa_to_gfn(~0ULL));
> +		srcu_read_unlock(&kvm->srcu, idx);
> +	}
>  }
>  
>  void kvm_arch_register_noncoherent_dma(struct kvm *kvm)
> -- 
> 2.43.0

Re: [PATCH] KVM: x86/mmu: Protect noncoherent DMA zaps with SRCU

[Huang, Kai]

On Mon, 2026-08-24 at 10:49 -0700, Sean Christopherson wrote:
> Alternatively, what if we have kvm_zap_gfn_range() acquire SRCU?  Nesting "locks"
> is a-ok, and two of the three users of kvm_zap_gfn_range() have had this bug (see
> commit 074c00800719 ("KVM: x86: Use SRCU to protect zap in
> __kvm_set_or_clear_apicv_inhibit()").

kvm_vcpu_srcu_read_lock() doesn't allow nesting lock of KVM's SRCU.  Doesn't
seem to be a problem for this particular case, though, because
sev_handle_rmp_fault() calls kvm_zap_gfn_range() after kvm_vcpu_srcu_read_lock()
is called.

Re: [PATCH] KVM: x86/mmu: Protect noncoherent DMA zaps with SRCU

[Sean Christopherson]

On Mon, Aug 24, 2026, Kai Huang wrote:
> On Mon, 2026-08-24 at 10:49 -0700, Sean Christopherson wrote:
> > Alternatively, what if we have kvm_zap_gfn_range() acquire SRCU?  Nesting "locks"
> > is a-ok, and two of the three users of kvm_zap_gfn_range() have had this bug (see
> > commit 074c00800719 ("KVM: x86: Use SRCU to protect zap in
> > __kvm_set_or_clear_apicv_inhibit()").
> 
> kvm_vcpu_srcu_read_lock() doesn't allow nesting lock of KVM's SRCU.

No, kvm_vcpu_srcu_read_lock() doesn't allow nested usage of itself.  Commit
2031f2876896 ("KVM: Add helpers to wrap vcpu->srcu_idx and yell if it's abused")
added the protection because we had multiple bugs where KVM would clobber
vcpu->srcu_idx and cause a deadlock by leaking a grace period reference.

> Doesn't seem to be a problem for this particular case, though, because
> sev_handle_rmp_fault() calls kvm_zap_gfn_range() after
> kvm_vcpu_srcu_read_lock() is called.

kvm_vcpu_srcu_read_lock() can nested inside an existing kvm->srcu critical section,
just so long as the existing critical section isn't tracking its grace period via
vcpu->____srcu_idx.

Re: [PATCH] KVM: x86/mmu: Protect noncoherent DMA zaps with SRCU

[Huang, Kai]

On Mon, 2026-08-24 at 14:49 -0700, Sean Christopherson wrote:
> On Mon, Aug 24, 2026, Kai Huang wrote:
> > On Mon, 2026-08-24 at 10:49 -0700, Sean Christopherson wrote:
> > > Alternatively, what if we have kvm_zap_gfn_range() acquire SRCU?  Nesting "locks"
> > > is a-ok, and two of the three users of kvm_zap_gfn_range() have had this bug (see
> > > commit 074c00800719 ("KVM: x86: Use SRCU to protect zap in
> > > __kvm_set_or_clear_apicv_inhibit()").
> > 
> > kvm_vcpu_srcu_read_lock() doesn't allow nesting lock of KVM's SRCU.
> 
> No, kvm_vcpu_srcu_read_lock() doesn't allow nested usage of itself.  Commit
> 2031f2876896 ("KVM: Add helpers to wrap vcpu->srcu_idx and yell if it's abused")
> added the protection because we had multiple bugs where KVM would clobber
> vcpu->srcu_idx and cause a deadlock by leaking a grace period reference.
> 
> > Doesn't seem to be a problem for this particular case, though, because
> > sev_handle_rmp_fault() calls kvm_zap_gfn_range() after
> > kvm_vcpu_srcu_read_lock() is called.
> 
> kvm_vcpu_srcu_read_lock() can nested inside an existing kvm->srcu critical section,
> just so long as the existing critical section isn't tracking its grace period via
> vcpu->____srcu_idx.

Hmm right, doesn't seem calling kvm_vcpu_srcu_read_lock() inside existing kvm-
>srcu could clobber vcpu->____srcu_idx.  So agree it's safe to explicitly hold
SRCU read lock inside kvm_zap_gfn_range().