In a CoCo VM the host is untrusted. Since the VMBus ring buffer read and
write indices live in shared memory, the guest has to treat both as
potentially malicious.
hv_ringbuffer_write() copies into the ring at write_index with no bounds
checking, so the host can make the guest write packet data at any offset
up to 4 GiB past the start of the ring buffer. read_index matters too:
the available space derived from both indices is the only bound on how
much is copied, and an out-of-range index can make it far larger than
the ring.
The fix is to validate both indices before use and to use the validated
snapshot instead of re-accessing. For invalid indices,
hv_ringbuffer_write() logs and returns -EIO.
Open-coding the bytes available arithmetic keeps the fix free of
prerequisites; a later patch refactors it into a helper function.
The unchecked write goes back to the commit in the Fixes tag, but the
host is only untrusted in CoCo VMs, which Linux has supported since
v5.12, so the stable tag starts at 5.15.x. This applies as-is to v5.15
and later.
Fixes: 3e7ee4902fe6 ("Staging: hv: add the Hyper-V virtual bus")
Cc: <[email protected]> # 5.15.x
Signed-off-by: Kameron Carr <[email protected]>
---
drivers/hv/ring_buffer.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 592a960..a18b309 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -70,15 +70,6 @@ static void hv_signal_on_write(u32 old_write, struct
vmbus_channel *channel)
}
}
-/* Get the next write location for the specified ring buffer. */
-static inline u32
-hv_get_next_write_location(struct hv_ring_buffer_info *ring_info)
-{
- u32 next = ring_info->ring_buffer->write_index;
-
- return next;
-}
-
/* Set the next write location for the specified ring buffer. */
static inline void
hv_set_next_write_location(struct hv_ring_buffer_info *ring_info,
@@ -281,6 +272,7 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
u32 totalbytes_towrite = sizeof(u64);
u32 next_write_location;
u32 old_write;
+ u32 read_index;
u64 prev_indices;
unsigned long flags;
struct hv_ring_buffer_info *outring_info = &channel->outbound;
@@ -295,7 +287,20 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
spin_lock_irqsave(&outring_info->ring_lock, flags);
- bytes_avail_towrite = hv_get_bytes_to_write(outring_info);
+ read_index = READ_ONCE(outring_info->ring_buffer->read_index);
+ old_write = READ_ONCE(outring_info->ring_buffer->write_index);
+ if (unlikely(read_index >= outring_info->ring_datasize ||
+ old_write >= outring_info->ring_datasize)) {
+ spin_unlock_irqrestore(&outring_info->ring_lock, flags);
+ pr_err_ratelimited("outbound ring indices out of range: relid
%u read %u write %u size %u\n",
+ channel->offermsg.child_relid, read_index,
+ old_write, outring_info->ring_datasize);
+ return -EIO;
+ }
+
+ bytes_avail_towrite = old_write >= read_index ?
+ outring_info->ring_datasize - (old_write - read_index) :
+ read_index - old_write;
/*
* If there is only room for the packet, assume it is full.
@@ -317,9 +322,7 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
channel->out_full_flag = false;
/* Write to the ring buffer */
- next_write_location = hv_get_next_write_location(outring_info);
-
- old_write = next_write_location;
+ next_write_location = old_write;
for (i = 0; i < kv_count; i++) {
next_write_location = hv_copyto_ringbuffer(outring_info,
--
2.45.4
