[PATCH] net/virtio-user: fix control queue allocation for non-vDPA

[PATCH] net/virtio-user: fix control queue allocation for non-vDPA

[Maxime Coquelin]

For non-vDPA backends, where the backend does not support
control queue, it is still emulated in the Virtio-user
layer to handle multiqueue feature. The frontend setups a
control queue, which is hidden to the device. It means the
number of vrings metadata to allocate should be based on
the frontend features and not the device features.

This patch fixes out-of-range access reported by ASan,
which could sometimes be noticed at exit time by a
segmentation fault when disabled:

Fixes: b80947743f5e ("net/virtio-user: fix control queue allocation")

Reported-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index b2c6c2b7df..fed66d2ae9 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -624,7 +624,7 @@ virtio_user_alloc_vrings(struct virtio_user_dev *dev)
 	bool packed_ring = !!(dev->device_features & (1ull << VIRTIO_F_RING_PACKED));
 
 	nr_vrings = dev->max_queue_pairs * 2;
-	if (dev->device_features & (1ull << VIRTIO_NET_F_CTRL_VQ))
+	if (dev->frontend_features & (1ull << VIRTIO_NET_F_CTRL_VQ))
 		nr_vrings++;
 
 	dev->callfds = rte_zmalloc("virtio_user_dev", nr_vrings * sizeof(*dev->callfds), 0);
-- 
2.45.2

Re: [PATCH] net/virtio-user: fix control queue allocation for non-vDPA

[David Marchand]

On Wed, Jul 3, 2024 at 6:27 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> For non-vDPA backends, where the backend does not support
> control queue, it is still emulated in the Virtio-user
> layer to handle multiqueue feature. The frontend setups a
> control queue, which is hidden to the device. It means the
> number of vrings metadata to allocate should be based on
> the frontend features and not the device features.
>
> This patch fixes out-of-range access reported by ASan,
> which could sometimes be noticed at exit time by a
> segmentation fault when disabled:
>
> Fixes: b80947743f5e ("net/virtio-user: fix control queue allocation")
>
> Reported-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Reviewed-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

Re: [PATCH] net/virtio-user: fix control queue allocation for non-vDPA

[Maxime Coquelin]

On 7/3/24 18:27, Maxime Coquelin wrote:
> For non-vDPA backends, where the backend does not support
> control queue, it is still emulated in the Virtio-user
> layer to handle multiqueue feature. The frontend setups a
> control queue, which is hidden to the device. It means the
> number of vrings metadata to allocate should be based on
> the frontend features and not the device features.
> 
> This patch fixes out-of-range access reported by ASan,
> which could sometimes be noticed at exit time by a
> segmentation fault when disabled:
> 
> Fixes: b80947743f5e ("net/virtio-user: fix control queue allocation")
> 
> Reported-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index b2c6c2b7df..fed66d2ae9 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -624,7 +624,7 @@ virtio_user_alloc_vrings(struct virtio_user_dev *dev)
>   	bool packed_ring = !!(dev->device_features & (1ull << VIRTIO_F_RING_PACKED));
>   
>   	nr_vrings = dev->max_queue_pairs * 2;
> -	if (dev->device_features & (1ull << VIRTIO_NET_F_CTRL_VQ))
> +	if (dev->frontend_features & (1ull << VIRTIO_NET_F_CTRL_VQ))
>   		nr_vrings++;
>   
>   	dev->callfds = rte_zmalloc("virtio_user_dev", nr_vrings * sizeof(*dev->callfds), 0);

Applied to next-virtio/for-next-net.

Thanks for the review,
Maxime