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

[Michael Kelley]

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

Reviewed-by: Michael Kelley <mhklinux@xxxxxxxxxxx>

> ---
>  drivers/hv/ring_buffer.c | 15 +++++++++++++--
>  include/linux/hyperv.h   |  6 ++++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index 29edab9..b54a7d3 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,12 @@ 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_index_valid(rbi, 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;
> +	}
> 
>  	if (write_loc >= priv_read_loc)
>  		return write_loc - priv_read_loc;
> @@ -441,7 +448,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);
> @@ -590,6 +597,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_index_valid(rbi, write_index) ||
> +		     !hv_ringbuffer_index_valid(rbi, start_read_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 9d7d09c..fd61382 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -248,6 +248,9 @@ 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_index_valid(rbi, read_loc) ||
> +		     !hv_ringbuffer_index_valid(rbi, write_loc)))
> +		return 0;
> 
>  	return hv_ringbuffer_avail_read(rbi, read_loc, write_loc);
>  }
> @@ -258,6 +261,9 @@ 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_index_valid(rbi, read_loc) ||
> +		     !hv_ringbuffer_index_valid(rbi, write_loc)))
> +		return 0;
> 
>  	return hv_ringbuffer_avail_write(rbi, read_loc, write_loc);
>  }
> 
> --
> 2.45.4