Skip to content

Commit 530a8ba

Browse files
committed
KVM: Bound the number of dirty ring entries in a single reset at INT_MAX
Cap the number of ring entries that are reset in a single ioctl to INT_MAX to ensure userspace isn't confused by a wrap into negative space, and so that, in a truly pathological scenario, KVM doesn't miss a TLB flush due to the count wrapping to zero. While the size of the ring is fixed at 0x10000 entries and KVM (currently) supports at most 4096, userspace is allowed to harvest entries from the ring while the reset is in-progress, i.e. it's possible for the ring to always have harvested entries. Opportunistically return an actual error code from the helper so that a future fix to handle pending signals can gracefully return -EINTR. Drop the function comment now that the return code is a stanard 0/-errno (and because a future commit will add a proper lockdep assertion). Opportunistically drop a similarly stale comment for kvm_dirty_ring_push(). Cc: Peter Xu <peterx@redhat.com> Cc: Yan Zhao <yan.y.zhao@intel.com> Cc: Maxim Levitsky <mlevitsk@redhat.com> Cc: Binbin Wu <binbin.wu@linux.intel.com> Fixes: fb04a1e ("KVM: X86: Implement ring-based dirty memory tracking") Reviewed-by: James Houghton <jthoughton@google.com> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com> Reviewed-by: Yan Zhao <yan.y.zhao@intel.com> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/r/20250516213540.2546077-2-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 28224ef commit 530a8ba

File tree

3 files changed

+16
-21
lines changed

3 files changed

+16
-21
lines changed

include/linux/kvm_dirty_ring.h

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@ static inline int kvm_dirty_ring_alloc(struct kvm *kvm, struct kvm_dirty_ring *r
4949
}
5050

5151
static inline int kvm_dirty_ring_reset(struct kvm *kvm,
52-
struct kvm_dirty_ring *ring)
52+
struct kvm_dirty_ring *ring,
53+
int *nr_entries_reset)
5354
{
54-
return 0;
55+
return -ENOENT;
5556
}
5657

5758
static inline void kvm_dirty_ring_push(struct kvm_vcpu *vcpu,
@@ -77,17 +78,8 @@ bool kvm_arch_allow_write_without_running_vcpu(struct kvm *kvm);
7778
u32 kvm_dirty_ring_get_rsvd_entries(struct kvm *kvm);
7879
int kvm_dirty_ring_alloc(struct kvm *kvm, struct kvm_dirty_ring *ring,
7980
int index, u32 size);
80-
81-
/*
82-
* called with kvm->slots_lock held, returns the number of
83-
* processed pages.
84-
*/
85-
int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring);
86-
87-
/*
88-
* returns =0: successfully pushed
89-
* <0: unable to push, need to wait
90-
*/
81+
int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring,
82+
int *nr_entries_reset);
9183
void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, u32 slot, u64 offset);
9284

9385
bool kvm_dirty_ring_check_request(struct kvm_vcpu *vcpu);

virt/kvm/dirty_ring.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,19 +105,19 @@ static inline bool kvm_dirty_gfn_harvested(struct kvm_dirty_gfn *gfn)
105105
return smp_load_acquire(&gfn->flags) & KVM_DIRTY_GFN_F_RESET;
106106
}
107107

108-
int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring)
108+
int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring,
109+
int *nr_entries_reset)
109110
{
110111
u32 cur_slot, next_slot;
111112
u64 cur_offset, next_offset;
112113
unsigned long mask;
113-
int count = 0;
114114
struct kvm_dirty_gfn *entry;
115115
bool first_round = true;
116116

117117
/* This is only needed to make compilers happy */
118118
cur_slot = cur_offset = mask = 0;
119119

120-
while (true) {
120+
while (likely((*nr_entries_reset) < INT_MAX)) {
121121
entry = &ring->dirty_gfns[ring->reset_index & (ring->size - 1)];
122122

123123
if (!kvm_dirty_gfn_harvested(entry))
@@ -130,7 +130,7 @@ int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring)
130130
kvm_dirty_gfn_set_invalid(entry);
131131

132132
ring->reset_index++;
133-
count++;
133+
(*nr_entries_reset)++;
134134
/*
135135
* Try to coalesce the reset operations when the guest is
136136
* scanning pages in the same slot.
@@ -167,7 +167,7 @@ int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring)
167167

168168
trace_kvm_dirty_ring_reset(ring);
169169

170-
return count;
170+
return 0;
171171
}
172172

173173
void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, u32 slot, u64 offset)

virt/kvm/kvm_main.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4962,15 +4962,18 @@ static int kvm_vm_ioctl_reset_dirty_pages(struct kvm *kvm)
49624962
{
49634963
unsigned long i;
49644964
struct kvm_vcpu *vcpu;
4965-
int cleared = 0;
4965+
int cleared = 0, r;
49664966

49674967
if (!kvm->dirty_ring_size)
49684968
return -EINVAL;
49694969

49704970
mutex_lock(&kvm->slots_lock);
49714971

4972-
kvm_for_each_vcpu(i, vcpu, kvm)
4973-
cleared += kvm_dirty_ring_reset(vcpu->kvm, &vcpu->dirty_ring);
4972+
kvm_for_each_vcpu(i, vcpu, kvm) {
4973+
r = kvm_dirty_ring_reset(vcpu->kvm, &vcpu->dirty_ring, &cleared);
4974+
if (r)
4975+
break;
4976+
}
49744977

49754978
mutex_unlock(&kvm->slots_lock);
49764979

0 commit comments

Comments
 (0)