[PATCH v2 0/5] Drivers: hv: vmbus: Harden the ring buffer against a malicious host

[PATCH v2 0/5] Drivers: hv: vmbus: Harden the ring buffer against a malicious host

[Kameron Carr]

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 and as changing at any time. This patch series
adds bounds checking and reuses the validated indices instead of
re-accessing them.

Patch 1 contains the minimum security fix, and is the only patch in the
series intended to be backported. hv_ringbuffer_write() copies into the
ring at the write index, so a malicious host can make the guest write to
memory outside the ring buffer. This is reachable on any channel a CoCo
VM accepts.

Patches 2 and 3 add READ/WRITE_ONCE annotations and refactor the helper
functions to allow the caller to work with a consistent snapshot of the
ring buffer indices.

Patch 4 adds the rest of the bounds checking. The memcopy() in
hv_pkt_iter_avail() can only result in an out-of-bounds read if
rbi->pkt_buffer_size exceeds the ring's data size. KVP is the only
in-tree channel whose max_pkt_size exceeds its ring's data size (16K vs
12K on a 4K page guest). CoCo VMs reject the KVP channel, so this bug is
currently unreachable on CoCo VMs. The other paths patch 4 checks can't
cause a bad access, only a nonsense byte count or a wrong signaling
decision.

Patch 1 applies cleanly to v5.15 and later. The unchecked write goes
back to the original driver, but the host is only untrusted in CoCo VMs,
which Linux has supported since v5.12, so patch 1's Fixes tag points at
the original driver while its stable tag starts at 5.15.x.

---
v2:
  - hv_ringbuffer_index_valid() -> hv_ringbuffer_indices_invalid()
    combines read and write checks and reverses the polarity
  - hv_pkt_iter_avail() reuses hv_ringbuffer_avail_read() arithmetic
  - Document the need to validate ringbuffer indices

v1: https://lore.kernel.org/all/20261001221040.1794904-1-kameroncarr@linux.microsoft.com/

Kameron Carr (5):
      Drivers: hv: vmbus: Bounds check the shared ring buffer indices
      Drivers: hv: vmbus: Use *_ONCE() to access shared ring buffer indices
      Drivers: hv: vmbus: Compute ring byte counts from a caller-held snapshot
      Drivers: hv: vmbus: Keep the ring byte counts sane for a bad index
      Documentation: hyperv: Describe validation of the ring buffer indices

 Documentation/virt/hyperv/coco.rst |   5 +-
 drivers/hv/ring_buffer.c           | 118 ++++++++++++++++---------------------
 include/linux/hyperv.h             |  58 ++++++++++++++----
 3 files changed, 100 insertions(+), 81 deletions(-)
---
base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e

[PATCH v2 1/5] Drivers: hv: vmbus: Bounds check the shared ring buffer indices

[Kameron Carr]

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: 3e7ee4902fe6 ("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>
---
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.

 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..a18b309c 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

[PATCH v2 2/5] Drivers: hv: vmbus: Use *_ONCE() to access shared ring buffer indices

[Kameron Carr]

The ring buffer read/write indices live in a page shared with the host,
so the compiler must not split, merge or refetch accesses to them. Add
READ_ONCE()/WRITE_ONCE() in hv_set_next_write_location(),
hv_pkt_iter_close(), hv_get_bytes_to_read() and hv_get_bytes_to_write().
The accesses in hv_ringbuffer_get_debuginfo() are left to the next
patch.

Drop hv_get_ring_bufferindices(). It has one caller and is a one-line
expression on the write index. Inlining it moves the access to the call
site, so hv_ringbuffer_write() can reuse its validated snapshot of that
index, old_write, rather than reading the shared memory a second time.

No functional change intended for a well-behaved host.

Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
---
 drivers/hv/ring_buffer.c | 15 ++++-----------
 include/linux/hyperv.h   |  4 ++--
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index a18b309c..7f466c5 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -75,7 +75,7 @@ static inline void
 hv_set_next_write_location(struct hv_ring_buffer_info *ring_info,
 		     u32 next_write_location)
 {
-	ring_info->ring_buffer->write_index = next_write_location;
+	WRITE_ONCE(ring_info->ring_buffer->write_index, next_write_location);
 }
 
 /* Get the size of the ring buffer. */
@@ -85,13 +85,6 @@ hv_get_ring_buffersize(const struct hv_ring_buffer_info *ring_info)
 	return ring_info->ring_datasize;
 }
 
-/* Get the read and write indices as u64 of the specified ring buffer. */
-static inline u64
-hv_get_ring_bufferindices(struct hv_ring_buffer_info *ring_info)
-{
-	return (u64)ring_info->ring_buffer->write_index << 32;
-}
-
 /*
  * Helper routine to copy from source to ring buffer.
  * Assume there is enough room. Handles wrap-around in dest case only!!
@@ -358,7 +351,7 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
 		*trans_id = __trans_id;
 
 	/* Set previous packet start */
-	prev_indices = hv_get_ring_bufferindices(outring_info);
+	prev_indices = (u64)old_write << 32;
 
 	next_write_location = hv_copyto_ringbuffer(outring_info,
 					     next_write_location,
@@ -582,8 +575,8 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
 	 * is updated.
 	 */
 	virt_rmb();
-	start_read_index = rbi->ring_buffer->read_index;
-	rbi->ring_buffer->read_index = rbi->priv_read_index;
+	start_read_index = READ_ONCE(rbi->ring_buffer->read_index);
+	WRITE_ONCE(rbi->ring_buffer->read_index, rbi->priv_read_index);
 
 	/*
 	 * Older versions of Hyper-V (before WS2102 and Win8) do not
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 9e109d9..5c65820 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -214,7 +214,7 @@ static inline u32 hv_get_bytes_to_read(const struct hv_ring_buffer_info *rbi)
 	u32 read_loc, write_loc, dsize, read;
 
 	dsize = rbi->ring_datasize;
-	read_loc = rbi->ring_buffer->read_index;
+	read_loc = READ_ONCE(rbi->ring_buffer->read_index);
 	write_loc = READ_ONCE(rbi->ring_buffer->write_index);
 
 	read = write_loc >= read_loc ? (write_loc - read_loc) :
@@ -229,7 +229,7 @@ static inline u32 hv_get_bytes_to_write(const struct hv_ring_buffer_info *rbi)
 
 	dsize = rbi->ring_datasize;
 	read_loc = READ_ONCE(rbi->ring_buffer->read_index);
-	write_loc = rbi->ring_buffer->write_index;
+	write_loc = READ_ONCE(rbi->ring_buffer->write_index);
 
 	write = write_loc >= read_loc ? dsize - (write_loc - read_loc) :
 		read_loc - write_loc;

-- 
2.45.4

[PATCH v2 3/5] Drivers: hv: vmbus: Compute ring byte counts from a caller-held snapshot

[Kameron Carr]

hv_get_ringbuffer_availbytes() reads the indices directly, so a caller
that also wants the index values has to read them a second time. The
host can change them in between, resulting in the byte counts and the
indices reflecting two different states of the ring.

Replace it with hv_ringbuffer_avail_write() and
hv_ringbuffer_avail_read(), which take the indices as arguments and
return a single count. They are separate because most callers want only
one of the two values. Use them in hv_ringbuffer_write() in place of the
open-coded equivalent. hv_get_bytes_to_read(), hv_get_bytes_to_write()
and hv_pkt_iter_avail() duplicated the same arithmetic, so put the
helpers in include/linux/hyperv.h and use them there too.

Add a hv_ringbuffer_indices_invalid() helper that bounds checks a pair
of read and write indices, and use it for the check in
hv_ringbuffer_write(). Convert the remaining callers to use a single
snapshot:

  - hv_ringbuffer_get_debuginfo() now reports the indices and the
    computed byte counts from the same snapshot.

  - hv_pkt_iter_close() computes the free space from priv_read_index
    instead of re-reading the shared read index. The two agree unless a
    misbehaving host has rewritten the value.

No functional change intended for a well-behaved host.

Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
---
 drivers/hv/ring_buffer.c | 68 ++++++++++++++++--------------------------------
 include/linux/hyperv.h   | 50 +++++++++++++++++++++++++++--------
 2 files changed, 61 insertions(+), 57 deletions(-)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 7f466c5..cda957d 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -107,35 +107,12 @@ static u32 hv_copyto_ringbuffer(
 	return start_write_offset;
 }
 
-/*
- *
- * hv_get_ringbuffer_availbytes()
- *
- * Get number of bytes available to read and to write to
- * for the specified ring buffer
- */
-static void
-hv_get_ringbuffer_availbytes(const struct hv_ring_buffer_info *rbi,
-			     u32 *read, u32 *write)
-{
-	u32 read_loc, write_loc, dsize;
-
-	/* Capture the read/write indices before they changed */
-	read_loc = READ_ONCE(rbi->ring_buffer->read_index);
-	write_loc = READ_ONCE(rbi->ring_buffer->write_index);
-	dsize = rbi->ring_datasize;
-
-	*write = write_loc >= read_loc ? dsize - (write_loc - read_loc) :
-		read_loc - write_loc;
-	*read = dsize - *write;
-}
-
 /* Get various debug metrics for the specified ring buffer. */
 int hv_ringbuffer_get_debuginfo(struct hv_ring_buffer_info *ring_info,
 				struct hv_ring_buffer_debug_info *debug_info)
 {
-	u32 bytes_avail_towrite;
-	u32 bytes_avail_toread;
+	u32 read_index;
+	u32 write_index;
 
 	mutex_lock(&ring_info->ring_buffer_mutex);
 
@@ -144,13 +121,14 @@ int hv_ringbuffer_get_debuginfo(struct hv_ring_buffer_info *ring_info,
 		return -EINVAL;
 	}
 
-	hv_get_ringbuffer_availbytes(ring_info,
-				     &bytes_avail_toread,
-				     &bytes_avail_towrite);
-	debug_info->bytes_avail_toread = bytes_avail_toread;
-	debug_info->bytes_avail_towrite = bytes_avail_towrite;
-	debug_info->current_read_index = ring_info->ring_buffer->read_index;
-	debug_info->current_write_index = ring_info->ring_buffer->write_index;
+	read_index = READ_ONCE(ring_info->ring_buffer->read_index);
+	write_index = READ_ONCE(ring_info->ring_buffer->write_index);
+	debug_info->bytes_avail_toread =
+		hv_ringbuffer_avail_read(ring_info, read_index, write_index);
+	debug_info->bytes_avail_towrite =
+		hv_ringbuffer_avail_write(ring_info, read_index, write_index);
+	debug_info->current_read_index = read_index;
+	debug_info->current_write_index = write_index;
 	debug_info->current_interrupt_mask
 		= ring_info->ring_buffer->interrupt_mask;
 	mutex_unlock(&ring_info->ring_buffer_mutex);
@@ -282,8 +260,8 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
 
 	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)) {
+	if (unlikely(hv_ringbuffer_indices_invalid(outring_info, read_index,
+						   old_write))) {
 		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,
@@ -291,9 +269,8 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
 		return -EIO;
 	}
 
-	bytes_avail_towrite = old_write >= read_index ?
-		outring_info->ring_datasize - (old_write - read_index) :
-		read_index - old_write;
+	bytes_avail_towrite = hv_ringbuffer_avail_write(outring_info, read_index,
+							old_write);
 
 	/*
 	 * If there is only room for the packet, assume it is full.
@@ -445,10 +422,7 @@ static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info *rbi)
 	 */
 	write_loc = virt_load_acquire(&rbi->ring_buffer->write_index);
 
-	if (write_loc >= priv_read_loc)
-		return write_loc - priv_read_loc;
-	else
-		return (rbi->ring_datasize - priv_read_loc) + write_loc;
+	return hv_ringbuffer_avail_read(rbi, priv_read_loc, write_loc);
 }
 
 /*
@@ -568,6 +542,7 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
 {
 	struct hv_ring_buffer_info *rbi = &channel->inbound;
 	u32 curr_write_sz, pending_sz, bytes_read, start_read_index;
+	u32 write_index;
 
 	/*
 	 * Make sure all reads are done before we update the read index since
@@ -607,11 +582,13 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
 		return;
 
 	/*
-	 * Ensure the read of write_index in hv_get_bytes_to_write()
-	 * happens after the read of pending_send_sz.
+	 * Ensure the read of write_index happens after the read of
+	 * pending_send_sz.
 	 */
 	virt_rmb();
-	curr_write_sz = hv_get_bytes_to_write(rbi);
+	write_index = READ_ONCE(rbi->ring_buffer->write_index);
+	curr_write_sz = hv_ringbuffer_avail_write(rbi, rbi->priv_read_index,
+						  write_index);
 	bytes_read = hv_pkt_iter_bytes_read(rbi, start_read_index);
 
 	/*
@@ -627,8 +604,7 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
 	 * Exactly filling the ring buffer is treated as "not enough
 	 * space". The ring buffer always must have at least one byte
 	 * empty so the empty and full conditions are distinguishable.
-	 * hv_get_bytes_to_write() doesn't fully tell the truth in
-	 * this regard.
+	 * curr_write_sz doesn't fully tell the truth in this regard.
 	 *
 	 * So first check if we were in the "enough free space" state
 	 * before we began the iteration. If so, the host was not
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 5c65820..09f25ad 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -209,31 +209,59 @@ struct hv_ring_buffer_info {
 };
 
 
+/*
+ * The indices live in memory shared with the untrusted host, so check a
+ * snapshot of them before using it as an offset or to compute a byte count.
+ */
+static inline bool
+hv_ringbuffer_indices_invalid(const struct hv_ring_buffer_info *rbi,
+			      u32 read_loc, u32 write_loc)
+{
+	return read_loc >= rbi->ring_datasize ||
+	       write_loc >= rbi->ring_datasize;
+}
+
+/*
+ * Byte counts for a caller-supplied snapshot of the indices, so that the
+ * counts and the indices the caller goes on to use describe one state of the
+ * ring.
+ */
+static inline u32
+hv_ringbuffer_avail_write(const struct hv_ring_buffer_info *rbi,
+			  u32 read_loc, u32 write_loc)
+{
+	u32 dsize = rbi->ring_datasize;
+
+	return write_loc >= read_loc ? dsize - (write_loc - read_loc) :
+		read_loc - write_loc;
+}
+
+static inline u32
+hv_ringbuffer_avail_read(const struct hv_ring_buffer_info *rbi,
+			 u32 read_loc, u32 write_loc)
+{
+	return rbi->ring_datasize -
+		hv_ringbuffer_avail_write(rbi, read_loc, write_loc);
+}
+
 static inline u32 hv_get_bytes_to_read(const struct hv_ring_buffer_info *rbi)
 {
-	u32 read_loc, write_loc, dsize, read;
+	u32 read_loc, write_loc;
 
-	dsize = rbi->ring_datasize;
 	read_loc = READ_ONCE(rbi->ring_buffer->read_index);
 	write_loc = READ_ONCE(rbi->ring_buffer->write_index);
 
-	read = write_loc >= read_loc ? (write_loc - read_loc) :
-		(dsize - read_loc) + write_loc;
-
-	return read;
+	return hv_ringbuffer_avail_read(rbi, read_loc, write_loc);
 }
 
 static inline u32 hv_get_bytes_to_write(const struct hv_ring_buffer_info *rbi)
 {
-	u32 read_loc, write_loc, dsize, write;
+	u32 read_loc, write_loc;
 
-	dsize = rbi->ring_datasize;
 	read_loc = READ_ONCE(rbi->ring_buffer->read_index);
 	write_loc = READ_ONCE(rbi->ring_buffer->write_index);
 
-	write = write_loc >= read_loc ? dsize - (write_loc - read_loc) :
-		read_loc - write_loc;
-	return write;
+	return hv_ringbuffer_avail_write(rbi, read_loc, write_loc);
 }
 
 static inline u32 hv_get_avail_to_write_percent(

-- 
2.45.4

[PATCH v2 4/5] Drivers: hv: vmbus: Keep the ring byte counts sane for a bad index

[Kameron Carr]

The remaining users of the shared indices can't cause a bad memory
access with the channels a CoCo VM accepts today, but an out-of-range
index gives them a nonsense byte count. Give them defined behavior
instead.

hv_pkt_iter_first() bounds its memcpy() by hv_pkt_iter_avail(), which
is derived from write_index, and by pkt_buffer_size, which comes from
max_pkt_size and can exceed ring_datasize. A bad write index can then
make it read past the end of the ring. Only KVP has such a max_pkt_size
(16K on a 12K ring with 4K pages), and vmbus_is_valid_offer() rejects
it in isolated VMs, so this is latent. hv_pkt_iter_avail() now reports
an empty ring for a bad write index and logs it, rate-limited like
hv_ringbuffer_write(). It takes the channel instead of the ring so the
message can include the relid.

hv_get_bytes_to_read() and hv_get_bytes_to_write() now return 0 for a
bad index, so callers see nothing to read and no room to write. This
also stops hv_end_read() from reporting data that hv_pkt_iter_first()
won't return, which would keep a channel rescheduling its callback.

hv_pkt_iter_close() only uses the indices to decide whether to signal
the host, so skip the signal if either is out of range.

Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
---
 drivers/hv/ring_buffer.c | 16 ++++++++++++++--
 include/linux/hyperv.h   |  4 ++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index cda957d..72f30ab 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -408,8 +408,9 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
  * This is similar to hv_get_bytes_to_read but with private
  * read index instead.
  */
-static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info *rbi)
+static u32 hv_pkt_iter_avail(const struct vmbus_channel *channel)
 {
+	const struct hv_ring_buffer_info *rbi = &channel->inbound;
 	u32 priv_read_loc = rbi->priv_read_index;
 	u32 write_loc;
 
@@ -421,6 +422,13 @@ static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info *rbi)
 	 * stale data.
 	 */
 	write_loc = virt_load_acquire(&rbi->ring_buffer->write_index);
+	if (unlikely(hv_ringbuffer_indices_invalid(rbi, priv_read_loc,
+						   write_loc))) {
+		pr_err_ratelimited("inbound write index out of range: relid %u write %u size %u\n",
+				   channel->offermsg.child_relid, write_loc,
+				   rbi->ring_datasize);
+		return 0;
+	}
 
 	return hv_ringbuffer_avail_read(rbi, priv_read_loc, write_loc);
 }
@@ -438,7 +446,7 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct vmbus_channel *channel)
 
 	hv_debug_delay_test(channel, MESSAGE_DELAY);
 
-	bytes_avail = hv_pkt_iter_avail(rbi);
+	bytes_avail = hv_pkt_iter_avail(channel);
 	if (bytes_avail < sizeof(struct vmpacket_descriptor))
 		return NULL;
 	bytes_avail = min(rbi->pkt_buffer_size, bytes_avail);
@@ -587,6 +595,10 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
 	 */
 	virt_rmb();
 	write_index = READ_ONCE(rbi->ring_buffer->write_index);
+	if (unlikely(hv_ringbuffer_indices_invalid(rbi, start_read_index,
+						   write_index)))
+		return;
+
 	curr_write_sz = hv_ringbuffer_avail_write(rbi, rbi->priv_read_index,
 						  write_index);
 	bytes_read = hv_pkt_iter_bytes_read(rbi, start_read_index);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 09f25ad..216c0b9 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -250,6 +250,8 @@ static inline u32 hv_get_bytes_to_read(const struct hv_ring_buffer_info *rbi)
 
 	read_loc = READ_ONCE(rbi->ring_buffer->read_index);
 	write_loc = READ_ONCE(rbi->ring_buffer->write_index);
+	if (unlikely(hv_ringbuffer_indices_invalid(rbi, read_loc, write_loc)))
+		return 0;
 
 	return hv_ringbuffer_avail_read(rbi, read_loc, write_loc);
 }
@@ -260,6 +262,8 @@ static inline u32 hv_get_bytes_to_write(const struct hv_ring_buffer_info *rbi)
 
 	read_loc = READ_ONCE(rbi->ring_buffer->read_index);
 	write_loc = READ_ONCE(rbi->ring_buffer->write_index);
+	if (unlikely(hv_ringbuffer_indices_invalid(rbi, read_loc, write_loc)))
+		return 0;
 
 	return hv_ringbuffer_avail_write(rbi, read_loc, write_loc);
 }

-- 
2.45.4

[PATCH v2 5/5] Documentation: hyperv: Describe validation of the ring buffer indices

[Kameron Carr]

Document that the guest must snapshot and validate the read and write
indices before using them to index into the ring buffer or to compute
the available space.

Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
---
 Documentation/virt/hyperv/coco.rst | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/virt/hyperv/coco.rst b/Documentation/virt/hyperv/coco.rst
index 3231e51..3e20249 100644
--- a/Documentation/virt/hyperv/coco.rst
+++ b/Documentation/virt/hyperv/coco.rst
@@ -200,7 +200,10 @@ validation is performed. For messages passed from the host to the guest in a
 VMBus ring buffer, the length of the message is validated, and the message is
 copied into a temporary (encrypted) buffer for further validation and
 processing. The copying adds a small amount of overhead, but is the only way
-to protect against a malicious host. See hv_pkt_iter_first().
+to protect against a malicious host. See hv_pkt_iter_first(). Similarly, the
+ring buffer header is shared with the host, so the guest must snapshot and
+validate the read and write indices before using them to index into the ring
+buffer or to compute available space.
 
 Many drivers for VMBus devices have been "hardened" by adding code to fully
 validate messages received over VMBus, instead of assuming that Hyper-V is

-- 
2.45.4