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

[Michael Kelley]

From: Kameron Carr <kameroncarr@xxxxxxxxxxxxxxxxxxx> Sent: Thursday, October 1, 2026 3:11 PM
> 
> 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.

I worked quite a bit on VMBus hardening for CoCo VMs back in 2021 and
2022. When I first saw the Subject of your patch set, I was a bit incredulous.
Surely we had not missed this rather obvious vulnerability! But we certainly
did, and I can't help but be a little embarrassed. :-( Thanks for putting things right.

I think this topic deserves a mention in Documentation/virt/hyperv/coco.rst.
There's a section entitled "Guest communication with Hyper-V" with
this paragraph:

   Similarly, when the guest reads memory that is shared with the host, it must
   validate the data before acting on it so that a malicious host cannot induce
   the guest to expose unintended data. Doing such validation can be tricky
   because the host can modify the shared memory areas even while or after
   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().

This documentation is only a high-level overview, but maybe add a sentence
such as:

  Also, because the ring buffer header is shared with the host, the guest
  must validate the read and indices before indexing into the ring buffer to
  send or receive messages, or when using them to compute available space.

Overall this series is well done. I have only a couple of minor quibbles
or suggestions on individual patches, and I've given my "Reviewed-by" on
all patches.

Michael

> 
> 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.
> 
> ---
> Kameron Carr (4):
>       Drivers: hv: vmbus: Bounds check the shared ring buffer indices
>       Drivers: hv: vmbus: Annotate accesses to the 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
> 
>  drivers/hv/ring_buffer.c | 112 +++++++++++++++++++++--------------------------
>  include/linux/hyperv.h   |  58 ++++++++++++++++++------
>  2 files changed, 94 insertions(+), 76 deletions(-)
> ---
> base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e