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

Message ID 20240703162738.283162-1-maxime.coquelin@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series net/virtio-user: fix control queue allocation for non-vDPA |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS

Commit Message

Maxime Coquelin July 3, 2024, 4:27 p.m. UTC
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(-)
  

Comments

David Marchand July 4, 2024, 9:25 a.m. UTC | #1
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>
  
Maxime Coquelin July 5, 2024, 7:50 a.m. UTC | #2
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
  

Patch

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);