stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full
       [not found] <20261110110724.3614412-1-fuad.tabba@linux.dev>
@ 2026-11-10 11:07 ` Fuad Tabba
  2026-11-10 14:22   ` Lorenzo Stoakes (ARM)
  2026-11-10 15:48   ` Sean Christopherson
  0 siblings, 2 replies; 5+ messages in thread
From: Fuad Tabba @ 2026-11-10 11:07 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson
  Cc: kvm, linux-kselftest, linux-doc, linux-kernel, Jonathan Corbet,
	Shuah Khan, Isaku Yamahata, Rick Edgecombe, Lorenzo Stoakes,
	Marc Zyngier, Oliver Upton, kvmarm, Wei-Lin Chang, Will Deacon,
	Fuad Tabba, stable

Pre-faulting can map a dirty-logged page writable, which pushes it to
the vCPU's dirty ring. KVM_RUN exits with KVM_EXIT_DIRTY_RING_FULL
before the ring overflows, but the pre-fault loop never checks it, so
one call over more such pages than the ring has free trips the WARN in
kvm_dirty_ring_push() and overwrites entries userspace hasn't
harvested. On x86:

  WARNING: arch/x86/kvm/../../../virt/kvm/dirty_ring.c:225 at kvm_dirty_ring_push+0x7a/0xd0
  Call Trace:
   make_spte+0x2cc/0x450
   kvm_tdp_mmu_map+0x5e1/0x7f0
   kvm_tdp_page_fault+0x12c/0x1b0
   kvm_mmu_do_page_fault+0x181/0x250
   kvm_arch_vcpu_pre_fault_memory+0x136/0x1d0
   kvm_vcpu_pre_fault_memory+0xcf/0x180
   kvm_vcpu_ioctl+0x455/0x770

Stop the loop once the ring is soft full, returning the pages already
processed, or -ENOSPC when there are none, since a retry can't succeed
until userspace harvests the ring. kvm_dirty_ring_check_request() fills
in a KVM_RUN exit, so test the ring's state directly. The stub for
!HAVE_KVM_DIRTY_RING is for s390, which has the ioctl but no ring.

Fixes: bc1a5cd002116 ("KVM: Add KVM_PRE_FAULT_MEMORY vcpu ioctl to pre-populate guest memory")
Cc: stable@vger.kernel.org
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 Documentation/virt/kvm/api.rst | 9 +++++++--
 include/linux/kvm_dirty_ring.h | 6 ++++++
 virt/kvm/dirty_ring.c          | 2 +-
 virt/kvm/kvm_main.c            | 6 ++++++
 4 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index e95c9cda085d9..89eb6be9bc6e3 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6544,6 +6544,7 @@ Errors:
   ENOENT     The specified `gpa` is outside defined memslots.
   ENOEXEC    The vCPU has not been initialised (arm64).
   EINTR      An unmasked signal is pending and no page was processed.
+  ENOSPC     The vCPU's dirty ring is full and no page was processed.
   EFAULT     The parameter address was invalid.
   EHWPOISON  A poisoned host page was encountered.
   EOPNOTSUPP Mapping memory for a GPA is unsupported by the
@@ -6588,7 +6589,9 @@ case, the ioctl can be called in parallel.
 
 When the ioctl returns, the input values are updated to point to the
 remaining range.  If `size` > 0 on return, the caller can just issue
-the ioctl again with the same `struct kvm_map_memory` argument.
+the ioctl again with the same `struct kvm_pre_fault_memory` argument.
+If that fails with ENOSPC, the vCPU's dirty ring has to be harvested
+and reset first.
 
 On x86, shadow page tables cannot support this ioctl because they
 are indexed by virtual address or nested guest physical address.
@@ -8957,7 +8960,9 @@ Therefore, the ioctl must be called *before* reading the content of
 the dirty pages.
 
 The dirty ring can get full.  When it happens, the KVM_RUN of the
-vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL.
+vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL, and
+KVM_PRE_FAULT_MEMORY on the vcpu will process no more pages until the
+ring has been harvested and reset.
 
 The dirty ring interface has a major difference comparing to the
 KVM_GET_DIRTY_LOG interface in that, when reading the dirty ring from
diff --git a/include/linux/kvm_dirty_ring.h b/include/linux/kvm_dirty_ring.h
index eb10d87adf7d5..88b4f2a009274 100644
--- a/include/linux/kvm_dirty_ring.h
+++ b/include/linux/kvm_dirty_ring.h
@@ -60,6 +60,11 @@ static inline void kvm_dirty_ring_push(struct kvm_vcpu *vcpu,
 {
 }
 
+static inline bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
+{
+	return false;
+}
+
 static inline struct page *kvm_dirty_ring_get_page(struct kvm_dirty_ring *ring,
 						   u32 offset)
 {
@@ -81,6 +86,7 @@ int kvm_dirty_ring_alloc(struct kvm *kvm, struct kvm_dirty_ring *ring,
 int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring,
 			 int *nr_entries_reset);
 void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, u32 slot, u64 offset);
+bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring);
 
 bool kvm_dirty_ring_check_request(struct kvm_vcpu *vcpu);
 
diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c
index 572b854edf740..f73f92102cb7a 100644
--- a/virt/kvm/dirty_ring.c
+++ b/virt/kvm/dirty_ring.c
@@ -40,7 +40,7 @@ static u32 kvm_dirty_ring_used(struct kvm_dirty_ring *ring)
 	return READ_ONCE(ring->dirty_index) - READ_ONCE(ring->reset_index);
 }
 
-static bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
+bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
 {
 	return kvm_dirty_ring_used(ring) >= ring->soft_limit;
 }
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf4cb586f1407..da1b76ceb65e3 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
 			break;
 		}
 
+		/* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */
+		if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {
+			r = -ENOSPC;
+			break;
+		}
+
 		r = kvm_arch_vcpu_pre_fault_memory(vcpu, range);
 		if (WARN_ON_ONCE(r == 0 || r == -EIO))
 			break;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full
  2026-11-10 11:07 ` [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full Fuad Tabba
@ 2026-11-10 14:22   ` Lorenzo Stoakes (ARM)
  2026-11-10 15:48   ` Sean Christopherson
  1 sibling, 0 replies; 5+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-10 14:22 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Paolo Bonzini, Sean Christopherson, kvm, linux-kselftest,
	linux-doc, linux-kernel, Jonathan Corbet, Shuah Khan,
	Isaku Yamahata, Rick Edgecombe, Marc Zyngier, Oliver Upton,
	kvmarm, Wei-Lin Chang, Will Deacon, Fuad Tabba, stable

On Tue, Nov 10, 2026 at 11:07:23AM +0000, Fuad Tabba wrote:
> Pre-faulting can map a dirty-logged page writable, which pushes it to
> the vCPU's dirty ring. KVM_RUN exits with KVM_EXIT_DIRTY_RING_FULL
> before the ring overflows, but the pre-fault loop never checks it, so
> one call over more such pages than the ring has free trips the WARN in
> kvm_dirty_ring_push() and overwrites entries userspace hasn't
> harvested. On x86:
>
>   WARNING: arch/x86/kvm/../../../virt/kvm/dirty_ring.c:225 at kvm_dirty_ring_push+0x7a/0xd0
>   Call Trace:
>    make_spte+0x2cc/0x450
>    kvm_tdp_mmu_map+0x5e1/0x7f0
>    kvm_tdp_page_fault+0x12c/0x1b0
>    kvm_mmu_do_page_fault+0x181/0x250
>    kvm_arch_vcpu_pre_fault_memory+0x136/0x1d0
>    kvm_vcpu_pre_fault_memory+0xcf/0x180
>    kvm_vcpu_ioctl+0x455/0x770

If you have a local repro maybe worth sending a regression test for this?... and
I type that, then check the 2/2 and see that you already did :) ok haha thanks!

>
> Stop the loop once the ring is soft full, returning the pages already
> processed, or -ENOSPC when there are none, since a retry can't succeed
> until userspace harvests the ring. kvm_dirty_ring_check_request() fills
> in a KVM_RUN exit, so test the ring's state directly. The stub for
> !HAVE_KVM_DIRTY_RING is for s390, which has the ioctl but no ring.
>
> Fixes: bc1a5cd002116 ("KVM: Add KVM_PRE_FAULT_MEMORY vcpu ioctl to pre-populate guest memory")
> Cc: stable@vger.kernel.org
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>

Yikes, nasty. Thanks for addressing this!

LGTM so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  Documentation/virt/kvm/api.rst | 9 +++++++--
>  include/linux/kvm_dirty_ring.h | 6 ++++++
>  virt/kvm/dirty_ring.c          | 2 +-
>  virt/kvm/kvm_main.c            | 6 ++++++
>  4 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index e95c9cda085d9..89eb6be9bc6e3 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6544,6 +6544,7 @@ Errors:
>    ENOENT     The specified `gpa` is outside defined memslots.
>    ENOEXEC    The vCPU has not been initialised (arm64).
>    EINTR      An unmasked signal is pending and no page was processed.
> +  ENOSPC     The vCPU's dirty ring is full and no page was processed.
>    EFAULT     The parameter address was invalid.
>    EHWPOISON  A poisoned host page was encountered.
>    EOPNOTSUPP Mapping memory for a GPA is unsupported by the
> @@ -6588,7 +6589,9 @@ case, the ioctl can be called in parallel.
>
>  When the ioctl returns, the input values are updated to point to the
>  remaining range.  If `size` > 0 on return, the caller can just issue
> -the ioctl again with the same `struct kvm_map_memory` argument.
> +the ioctl again with the same `struct kvm_pre_fault_memory` argument.
> +If that fails with ENOSPC, the vCPU's dirty ring has to be harvested
> +and reset first.
>
>  On x86, shadow page tables cannot support this ioctl because they
>  are indexed by virtual address or nested guest physical address.
> @@ -8957,7 +8960,9 @@ Therefore, the ioctl must be called *before* reading the content of
>  the dirty pages.
>
>  The dirty ring can get full.  When it happens, the KVM_RUN of the
> -vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL.
> +vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL, and
> +KVM_PRE_FAULT_MEMORY on the vcpu will process no more pages until the
> +ring has been harvested and reset.
>
>  The dirty ring interface has a major difference comparing to the
>  KVM_GET_DIRTY_LOG interface in that, when reading the dirty ring from
> diff --git a/include/linux/kvm_dirty_ring.h b/include/linux/kvm_dirty_ring.h
> index eb10d87adf7d5..88b4f2a009274 100644
> --- a/include/linux/kvm_dirty_ring.h
> +++ b/include/linux/kvm_dirty_ring.h
> @@ -60,6 +60,11 @@ static inline void kvm_dirty_ring_push(struct kvm_vcpu *vcpu,
>  {
>  }
>
> +static inline bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
> +{
> +	return false;
> +}
> +
>  static inline struct page *kvm_dirty_ring_get_page(struct kvm_dirty_ring *ring,
>  						   u32 offset)
>  {
> @@ -81,6 +86,7 @@ int kvm_dirty_ring_alloc(struct kvm *kvm, struct kvm_dirty_ring *ring,
>  int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring,
>  			 int *nr_entries_reset);
>  void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, u32 slot, u64 offset);
> +bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring);
>
>  bool kvm_dirty_ring_check_request(struct kvm_vcpu *vcpu);
>
> diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c
> index 572b854edf740..f73f92102cb7a 100644
> --- a/virt/kvm/dirty_ring.c
> +++ b/virt/kvm/dirty_ring.c
> @@ -40,7 +40,7 @@ static u32 kvm_dirty_ring_used(struct kvm_dirty_ring *ring)
>  	return READ_ONCE(ring->dirty_index) - READ_ONCE(ring->reset_index);
>  }
>
> -static bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
> +bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
>  {
>  	return kvm_dirty_ring_used(ring) >= ring->soft_limit;
>  }
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index cf4cb586f1407..da1b76ceb65e3 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
>  			break;
>  		}
>
> +		/* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */
> +		if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {
> +			r = -ENOSPC;
> +			break;
> +		}
> +
>  		r = kvm_arch_vcpu_pre_fault_memory(vcpu, range);
>  		if (WARN_ON_ONCE(r == 0 || r == -EIO))
>  			break;
> --
> 2.47.3
>

--
Cheers, Lorenzo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full
  2026-11-10 11:07 ` [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full Fuad Tabba
  2026-11-10 14:22   ` Lorenzo Stoakes (ARM)
@ 2026-11-10 15:48   ` Sean Christopherson
  2026-11-10 17:18     ` Fuad Tabba
  1 sibling, 1 reply; 5+ messages in thread
From: Sean Christopherson @ 2026-11-10 15:48 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Paolo Bonzini, kvm, linux-kselftest, linux-doc, linux-kernel,
	Jonathan Corbet, Shuah Khan, Isaku Yamahata, Rick Edgecombe,
	Lorenzo Stoakes, Marc Zyngier, Oliver Upton, kvmarm,
	Wei-Lin Chang, Will Deacon, Fuad Tabba, stable

On Tue, Nov 10, 2026, Fuad Tabba wrote:
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index cf4cb586f1407..da1b76ceb65e3 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
>  			break;
>  		}
>  
> +		/* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */

Please don't include AI comments verbatim.

> +		if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {

Eh, I vote to just use kvm_dirty_ring_check_request(), I don't see any harm in
clobbering vcpu->run->exit_reason.

Side topic, kvm_dirty_ring_check_request() is rather confusing (clears and then
re-sets the requests).  AFAICT, it can be this instead:

	lockdep_assert_held(vcpu->mutex);

	/*
	 * The vCPU isn't runnable when the dirty ring becomes soft full.  Keep
	 * the requests set until the dirty pages are harvested and the dirty
	 * ring is reset by userspace.  Note, checking if the ring is soft-full
	 * can get a false positive, but not a false negative, because only the
	 * vCPU can add entries to the ring, and its mutex is held.
	 */
	if (kvm_test_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu) &&
	    kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {
		vcpu->run->exit_reason = KVM_EXIT_DIRTY_RING_FULL;
		trace_kvm_dirty_ring_exit(vcpu);
		return true;
	}

	kvm_clear_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu);
	return false;

> +			r = -ENOSPC;
> +			break;
> +		}
> +
>  		r = kvm_arch_vcpu_pre_fault_memory(vcpu, range);
>  		if (WARN_ON_ONCE(r == 0 || r == -EIO))
>  			break;
> -- 
> 2.47.3
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full
  2026-11-10 15:48   ` Sean Christopherson
@ 2026-11-10 17:18     ` Fuad Tabba
  2026-11-10 18:35       ` Sean Christopherson
  0 siblings, 1 reply; 5+ messages in thread
From: Fuad Tabba @ 2026-11-10 17:18 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, kvm, linux-kselftest, linux-doc, linux-kernel,
	Jonathan Corbet, Shuah Khan, Isaku Yamahata, Rick Edgecombe,
	Lorenzo Stoakes, Marc Zyngier, Oliver Upton, kvmarm,
	Wei-Lin Chang, Will Deacon, stable

Hi Sean,

On Tue, 10 Nov 2026 15:48:37 +0000, Sean Christopherson
<seanjc@google.com> wrote:
> On Tue, Nov 10, 2026, Fuad Tabba wrote:
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
[...]
> > @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
> >                          break;
> >                  }
> >
> > +                /* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */
>
> Please don't include AI comments verbatim.

The series is LLM-assisted, but the comment was actually me.

> > +                if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {
>
> Eh, I vote to just use kvm_dirty_ring_check_request(), I don't see any harm in
> clobbering vcpu->run->exit_reason.

Will do.

> Side topic, kvm_dirty_ring_check_request() is rather confusing (clears and then
> re-sets the requests).  AFAICT, it can be this instead:
>
>         lockdep_assert_held(vcpu->mutex);
>
>         /*
>          * The vCPU isn't runnable when the dirty ring becomes soft full.  Keep
>          * the requests set until the dirty pages are harvested and the dirty
>          * ring is reset by userspace.  Note, checking if the ring is soft-full
>          * can get a false positive, but not a false negative, because only the
>          * vCPU can add entries to the ring, and its mutex is held.
>          */
>         if (kvm_test_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu) &&
>             kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {
>                 vcpu->run->exit_reason = KVM_EXIT_DIRTY_RING_FULL;
>                 trace_kvm_dirty_ring_exit(vcpu);
>                 return true;
>         }
>
>         kvm_clear_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu);
>         return false;

I'll send it as its own patch.

Cheers,
/fuad

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full
  2026-11-10 17:18     ` Fuad Tabba
@ 2026-11-10 18:35       ` Sean Christopherson
  0 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2026-11-10 18:35 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Paolo Bonzini, kvm, linux-kselftest, linux-doc, linux-kernel,
	Jonathan Corbet, Shuah Khan, Isaku Yamahata, Rick Edgecombe,
	Lorenzo Stoakes, Marc Zyngier, Oliver Upton, kvmarm,
	Wei-Lin Chang, Will Deacon, stable

On Tue, Nov 10, 2026, Fuad Tabba wrote:
> Hi Sean,
> 
> On Tue, 10 Nov 2026 15:48:37 +0000, Sean Christopherson
> <seanjc@google.com> wrote:
> > On Tue, Nov 10, 2026, Fuad Tabba wrote:
> > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> [...]
> > > @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
> > >                          break;
> > >                  }
> > >
> > > +                /* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */
> >
> > Please don't include AI comments verbatim.
> 
> The series is LLM-assisted, but the comment was actually me.

LOL, I think all of us are getting a bit corrupted.  :-)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-10 18:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20261110110724.3614412-1-fuad.tabba@linux.dev>
2026-11-10 11:07 ` [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full Fuad Tabba
2026-11-10 14:22   ` Lorenzo Stoakes (ARM)
2026-11-10 15:48   ` Sean Christopherson
2026-11-10 17:18     ` Fuad Tabba
2026-11-10 18:35       ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).