[PATCH 1/2] KVM: Flush dirty ring resets before handling signals

[PATCH 1/2] KVM: Flush dirty ring resets before handling signals

[Hao Zhang]

From: Hao Zhang <zhanghao1@kylinos.cn>

KVM bails out of KVM_RESET_DIRTY_RINGS if a signal is pending, but the
dirty ring reset flow can already have consumed harvested entries before
noticing the signal.  In that case, the entries have been marked invalid
and reset_index has been advanced, but the batched GFNs may not yet have
been reprotected by kvm_reset_dirty_gfn().

This breaks the dirty ring state machine: userspace has handed the GFNs
back to KVM, but KVM can leave the corresponding pages writable.  Future
guest writes to those pages may then fail to generate new dirty-ring
entries, causing dirty tracking to miss updates.

Make the signal path break out of the scan loop instead of returning
immediately, so any pending batch is flushed before kvm_dirty_ring_reset()
returns.

Fixes: 49005a2a3d2a ("KVM: Bail from the dirty ring reset flow if a signal is pending")
Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
---
 virt/kvm/dirty_ring.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c
index 572b854edf74..451eee9d8f69 100644
--- a/virt/kvm/dirty_ring.c
+++ b/virt/kvm/dirty_ring.c
@@ -122,6 +122,7 @@ int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring,
 	u64 cur_offset, next_offset;
 	unsigned long mask = 0;
 	struct kvm_dirty_gfn *entry;
+	int r = 0;
 
 	/*
 	 * Ensure concurrent calls to KVM_RESET_DIRTY_RINGS are serialized,
@@ -132,8 +133,10 @@ int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring,
 	lockdep_assert_held(&kvm->slots_lock);
 
 	while (likely((*nr_entries_reset) < INT_MAX)) {
-		if (signal_pending(current))
-			return -EINTR;
+		if (signal_pending(current)) {
+			r = -EINTR;
+			break;
+		}
 
 		entry = &ring->dirty_gfns[ring->reset_index & (ring->size - 1)];
 
@@ -213,7 +216,7 @@ int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring,
 
 	trace_kvm_dirty_ring_reset(ring);
 
-	return 0;
+	return r;
 }
 
 void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, u32 slot, u64 offset)

base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
-- 
2.15.0

[PATCH 2/2] KVM: Return dirty ring reset errors if no entries were reset

[Hao Zhang]

From: Hao Zhang <zhanghao1@kylinos.cn>

KVM_RESET_DIRTY_RINGS returns the number of entries that were reset.
Commit 530a8ba71b4c ("KVM: Bound the number of dirty ring entries in a
single reset at INT_MAX") converted kvm_dirty_ring_reset() to return a
standard 0/-errno, and commit 49005a2a3d2a ("KVM: Bail from the dirty
ring reset flow if a signal is pending") introduced an actual -EINTR path
by bailing out when a signal is pending.

kvm_vm_ioctl_reset_dirty_pages() currently breaks out of the vCPU loop on
error, but always returns the number of entries that were reset.  As a
result, KVM reports success with a return value of 0 if a signal is
already pending before any entries are reset.

Preserve the existing partial-success behavior by returning the number of
reset entries after forward progress, but propagate the error if no
entries were reset.

Fixes: 49005a2a3d2a ("KVM: Bail from the dirty ring reset flow if a signal is pending")
Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
---
 virt/kvm/kvm_main.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 65eb26a0520d..ad472c577a7a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4999,7 +4999,7 @@ static int kvm_vm_ioctl_reset_dirty_pages(struct kvm *kvm)
 {
 	unsigned long i;
 	struct kvm_vcpu *vcpu;
-	int cleared = 0, r;
+	int cleared = 0, r = 0;
 
 	if (!kvm->dirty_ring_size)
 		return -EINVAL;
@@ -5017,7 +5017,12 @@ static int kvm_vm_ioctl_reset_dirty_pages(struct kvm *kvm)
 	if (cleared)
 		kvm_flush_remote_tlbs(kvm);
 
-	return cleared;
+	/*
+	 * Preserve partial-success semantics if KVM made forward progress, but
+	 * don't squash errors when nothing was reset, e.g. if a signal was
+	 * already pending.
+	 */
+	return cleared ? cleared : r;
 }
 
 int __attribute__((weak)) kvm_vm_ioctl_enable_cap(struct kvm *kvm,
-- 
2.15.0