Re: [PATCH 1/2] virtio_ring: reject a split ring size that is not a power of two again

[Francesco Valla <francesco@valla.it>]

Hi Stjepan,

On Wed, Nov 11, 2026 at 06:29:37PM +0100, Stjepan Poljak wrote:
> The split ring indexes its avail and used rings with num - 1 as a mask,
> so its size must be a power of two. Commit 2a2d1382fe9d ("virtio: Add
> improved queue allocation API") moved that check from
> vring_new_virtqueue() into the new allocation path, which
> vring_new_virtqueue() does not use.
> 
> remoteproc calls vring_new_virtqueue() with a ring size from the
> firmware, checked only to be non-zero. With 100 entries, for example,
> the driver would put buffers 4 and 8 in avail slot 0. The device would
> look for them at slots 4 and 8, possibly reading the wrong data.
>

Good catch - I think I encountered something like this during my
experiments with virtio-over-remoteproc and ascribed it to a different
cause.

> Put the check back for the split ring; the packed ring has no such
> restriction.
> 
> Fixes: 2a2d1382fe9d ("virtio: Add improved queue allocation API")
> Signed-off-by: Stjepan Poljak <stjepan.poljak@gmail.com>
> ---
> remoteproc still reports this as -ENOMEM, although the "Bad virtqueue
> length" warning names the cause. Returning an error pointer from
> vring_new_virtqueue() is left for later; the same change for
> vring_create_virtqueue() is here:
> https://lore.kernel.org/r/20261106055635.3019889-1-stjepan.poljak@gmail.com
> ---
>  drivers/virtio/virtio_ring.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index f683a2d6a200..8da9bd0e70f4 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -3513,6 +3513,12 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
>  						    name, map);
>  	}
>  
> +	/* The split ring indexes its rings with num - 1 as a mask. */
> +	if (!is_power_of_2(num)) {
> +		dev_warn(&vdev->dev, "Bad virtqueue length %u\n", num);

This is definitely and error, and should be logged as such (I know, the
same message is printed as a warn in vring_alloc_queue_split(), but we
have no ABI to align to here).

Also, consider to adopt a message a bit more descriptive; maybe
something like this is sufficient:

   "Bad virtqueue length %u (not a power of two)\n"

> +		return NULL;
> +	}
> +
>  	vring_init(&vring_split.vring, num, pages, vring_align);
>  	return __vring_new_virtqueue_split(index, &vring_split, vdev, weak_barriers,
>  				     context, notify, callback, name,
> 
> base-commit: 1a577f9402c1780cc1e663f48d5dd54c02051b09
> -- 
> 2.53.0
> 
> 

Thank you!

Regards,
Francesco