mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
Drivers: hv: vmbus: Bounds check the shared ring buffer indices
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.
One re-read of write_index is left unchecked in this patch:
hv_get_ring_bufferindices() fetches it again for the prev_indices
trailer. The value is only returned to the host and is not used as an
offset or length for indexing. Since this is not a security issue, it is
addressed later in the series.
Open-coding the bytes available arithmetic keeps the fix free of
prerequisites; a later patch refactors it into a helper function.
Fixes: 3e7ee4902f ("Staging: hv: add the Hyper-V virtual bus")
Cc: stable@vger.kernel.org # 5.15.x
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
Signed-off-by: Wei Liu <wei.liu@kernel.org>
This commit is contained in:
parent
c03c1a1450
commit
1d287c17cc
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user