Skip to content

Commit e027ba1

Browse files
committed
KVM: Clean up coalesced MMIO ring full check
Fold coalesced_mmio_has_room() into its sole caller, coalesced_mmio_write(), as it's really just a single line of code, has a goofy return value, and is unnecessarily brittle. E.g. if coalesced_mmio_has_room() were to check ring->last directly, or the caller failed to use READ_ONCE(), KVM would be susceptible to TOCTOU attacks from userspace. Opportunistically add a comment explaining why on earth KVM leaves one entry free, which may not be obvious to readers that aren't familiar with ring buffers. No functional change intended. Reviewed-by: Ilias Stamatis <[email protected]> Cc: Paul Durrant <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Sean Christopherson <[email protected]>
1 parent 215b3cb commit e027ba1

File tree

1 file changed

+8
-21
lines changed

1 file changed

+8
-21
lines changed

virt/kvm/coalesced_mmio.c

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,25 +40,6 @@ static int coalesced_mmio_in_range(struct kvm_coalesced_mmio_dev *dev,
4040
return 1;
4141
}
4242

43-
static int coalesced_mmio_has_room(struct kvm_coalesced_mmio_dev *dev, u32 last)
44-
{
45-
struct kvm_coalesced_mmio_ring *ring;
46-
47-
/* Are we able to batch it ? */
48-
49-
/* last is the first free entry
50-
* check if we don't meet the first used entry
51-
* there is always one unused entry in the buffer
52-
*/
53-
ring = dev->kvm->coalesced_mmio_ring;
54-
if ((last + 1) % KVM_COALESCED_MMIO_MAX == READ_ONCE(ring->first)) {
55-
/* full */
56-
return 0;
57-
}
58-
59-
return 1;
60-
}
61-
6243
static int coalesced_mmio_write(struct kvm_vcpu *vcpu,
6344
struct kvm_io_device *this, gpa_t addr,
6445
int len, const void *val)
@@ -72,9 +53,15 @@ static int coalesced_mmio_write(struct kvm_vcpu *vcpu,
7253

7354
spin_lock(&dev->kvm->ring_lock);
7455

56+
/*
57+
* last is the index of the entry to fill. Verify userspace hasn't
58+
* set last to be out of range, and that there is room in the ring.
59+
* Leave one entry free in the ring so that userspace can differentiate
60+
* between an empty ring and a full ring.
61+
*/
7562
insert = READ_ONCE(ring->last);
76-
if (!coalesced_mmio_has_room(dev, insert) ||
77-
insert >= KVM_COALESCED_MMIO_MAX) {
63+
if (insert >= KVM_COALESCED_MMIO_MAX ||
64+
(insert + 1) % KVM_COALESCED_MMIO_MAX == READ_ONCE(ring->first)) {
7865
spin_unlock(&dev->kvm->ring_lock);
7966
return -EOPNOTSUPP;
8067
}

0 commit comments

Comments
 (0)