Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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 <ilstam@amazon.com>
Cc: Paul Durrant <paul@xen.org>
Link: https://lore.kernel.org/r/20240828181446.652474-3-seanjc@google.com
Signed-off-by: Sean Christopherson <seanjc@google.com>

+8 -21
+8 -21
virt/kvm/coalesced_mmio.c
··· 40 40 return 1; 41 41 } 42 42 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 - 62 43 static int coalesced_mmio_write(struct kvm_vcpu *vcpu, 63 44 struct kvm_io_device *this, gpa_t addr, 64 45 int len, const void *val) ··· 53 72 54 73 spin_lock(&dev->kvm->ring_lock); 55 74 75 + /* 76 + * last is the index of the entry to fill. Verify userspace hasn't 77 + * set last to be out of range, and that there is room in the ring. 78 + * Leave one entry free in the ring so that userspace can differentiate 79 + * between an empty ring and a full ring. 80 + */ 56 81 insert = READ_ONCE(ring->last); 57 - if (!coalesced_mmio_has_room(dev, insert) || 58 - insert >= KVM_COALESCED_MMIO_MAX) { 82 + if (insert >= KVM_COALESCED_MMIO_MAX || 83 + (insert + 1) % KVM_COALESCED_MMIO_MAX == READ_ONCE(ring->first)) { 59 84 spin_unlock(&dev->kvm->ring_lock); 60 85 return -EOPNOTSUPP; 61 86 }