From: Michael Kelley <mhklinux@outlook.com>
To: Kameron Carr <kameroncarr@linux.microsoft.com>,
Haiyang Zhang <haiyangz@microsoft.com>,
Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
Michael Kelley <mhklinux@outlook.com>
Cc: "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2 4/5] Drivers: hv: vmbus: Keep the ring byte counts sane for a bad index
Date: Tue, 20 Oct 2026 18:04:20 +0000 [thread overview]
Message-ID: <MN2PR02MB68635749E43264418EC2DB6CD4672@MN2PR02MB6863.namprd02.prod.outlook.com> (raw)
In-Reply-To: <20261019230950.1573677-5-kameroncarr@linux.microsoft.com>
From: Kameron Carr <kameroncarr@linux.microsoft.com> Sent: Monday, October 19, 2026 4:10 PM
>
> 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>
Reviewed-by: Michael Kelley <mhklinux@outlook.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
next prev parent reply other threads:[~2026-10-20 18:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-19 23:09 [PATCH v2 0/5] Drivers: hv: vmbus: Harden the ring buffer against a malicious host Kameron Carr
2026-10-19 23:09 ` [PATCH v2 1/5] Drivers: hv: vmbus: Bounds check the shared ring buffer indices Kameron Carr
2026-10-19 23:09 ` [PATCH v2 2/5] Drivers: hv: vmbus: Use *_ONCE() to access " Kameron Carr
2026-10-19 23:09 ` [PATCH v2 3/5] Drivers: hv: vmbus: Compute ring byte counts from a caller-held snapshot Kameron Carr
2026-10-20 18:04 ` Michael Kelley
2026-10-19 23:09 ` [PATCH v2 4/5] Drivers: hv: vmbus: Keep the ring byte counts sane for a bad index Kameron Carr
2026-10-20 18:04 ` Michael Kelley [this message]
2026-10-19 23:09 ` [PATCH v2 5/5] Documentation: hyperv: Describe validation of the ring buffer indices Kameron Carr
2026-10-20 18:04 ` Michael Kelley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN2PR02MB68635749E43264418EC2DB6CD4672@MN2PR02MB6863.namprd02.prod.outlook.com \
--to=mhklinux@outlook.com \
--cc=decui@microsoft.com \
--cc=haiyangz@microsoft.com \
--cc=kameroncarr@linux.microsoft.com \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=wei.liu@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®