[1/1] vhost: add NULL callback checks

Message ID 20250124141408.419674-2-ktraynor@redhat.com (mailing list archive)
State Awaiting Upstream
Delegated to: Maxime Coquelin
Headers
Series vhost: NULL callback checks |

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-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS

Commit Message

Kevin Traynor Jan. 24, 2025, 2:14 p.m. UTC
rte_vhost_driver_callback_register() does not specify
any mandatory callbacks in struct rte_vhost_device_ops.

Add some missing NULL checks before calling rte_vhost_device_ops
callbacks.

Fixes: 4796ad63ba1f ("examples/vhost: import userspace vhost application")
Cc: stable@dpdk.org

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
 lib/vhost/vduse.c      | 3 ++-
 lib/vhost/vhost.c      | 5 +++--
 lib/vhost/vhost_user.c | 3 ++-
 3 files changed, 7 insertions(+), 4 deletions(-)
  

Comments

Maxime Coquelin Feb. 7, 2025, 1:32 p.m. UTC | #1
On 1/24/25 3:14 PM, Kevin Traynor wrote:
> rte_vhost_driver_callback_register() does not specify
> any mandatory callbacks in struct rte_vhost_device_ops.
> 
> Add some missing NULL checks before calling rte_vhost_device_ops
> callbacks.
> 
> Fixes: 4796ad63ba1f ("examples/vhost: import userspace vhost application")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
>   lib/vhost/vduse.c      | 3 ++-
>   lib/vhost/vhost.c      | 5 +++--
>   lib/vhost/vhost_user.c | 3 ++-
>   3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
> index 8ba58555f9..be548f051e 100644
> --- a/lib/vhost/vduse.c
> +++ b/lib/vhost/vduse.c
> @@ -319,5 +319,6 @@ vduse_device_start(struct virtio_net *dev, bool reconnect)
>   	dev->flags |= VIRTIO_DEV_READY;
>   
> -	if (dev->notify_ops->new_device(dev->vid) == 0)
> +	if (!dev->notify_ops->new_device ||
> +		dev->notify_ops->new_device(dev->vid) == 0)
>   		dev->flags |= VIRTIO_DEV_RUNNING;
>   
> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> index 5a50a06f8d..c41c29ca57 100644
> --- a/lib/vhost/vhost.c
> +++ b/lib/vhost/vhost.c
> @@ -752,8 +752,9 @@ vhost_destroy_device_notify(struct virtio_net *dev)
>   	if (dev->flags & VIRTIO_DEV_RUNNING) {
>   		vdpa_dev = dev->vdpa_dev;
> -		if (vdpa_dev)
> +		if (vdpa_dev && vdpa_dev->ops->dev_close)
>   			vdpa_dev->ops->dev_close(dev->vid);
>   		dev->flags &= ~VIRTIO_DEV_RUNNING;
> -		dev->notify_ops->destroy_device(dev->vid);
> +		if (dev->notify_ops->destroy_device)
> +			dev->notify_ops->destroy_device(dev->vid);
>   	}
>   }
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 52d8078d7c..26dc0bde97 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -3302,5 +3302,6 @@ vhost_user_msg_handler(int vid, int fd)
>   
>   	if (!(dev->flags & VIRTIO_DEV_RUNNING)) {
> -		if (dev->notify_ops->new_device(dev->vid) == 0)
> +		if (!dev->notify_ops->new_device ||
> +			dev->notify_ops->new_device(dev->vid) == 0)
>   			dev->flags |= VIRTIO_DEV_RUNNING;
>   	}

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
  
Maxime Coquelin Feb. 7, 2025, 2:11 p.m. UTC | #2
On 1/24/25 3:14 PM, Kevin Traynor wrote:
> rte_vhost_driver_callback_register() does not specify
> any mandatory callbacks in struct rte_vhost_device_ops.
> 
> Add some missing NULL checks before calling rte_vhost_device_ops
> callbacks.
> 
> Fixes: 4796ad63ba1f ("examples/vhost: import userspace vhost application")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
>   lib/vhost/vduse.c      | 3 ++-
>   lib/vhost/vhost.c      | 5 +++--
>   lib/vhost/vhost_user.c | 3 ++-
>   3 files changed, 7 insertions(+), 4 deletions(-

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

Thanks,
Maxime
  

Patch

diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
index 8ba58555f9..be548f051e 100644
--- a/lib/vhost/vduse.c
+++ b/lib/vhost/vduse.c
@@ -319,5 +319,6 @@  vduse_device_start(struct virtio_net *dev, bool reconnect)
 	dev->flags |= VIRTIO_DEV_READY;
 
-	if (dev->notify_ops->new_device(dev->vid) == 0)
+	if (!dev->notify_ops->new_device ||
+		dev->notify_ops->new_device(dev->vid) == 0)
 		dev->flags |= VIRTIO_DEV_RUNNING;
 
diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 5a50a06f8d..c41c29ca57 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -752,8 +752,9 @@  vhost_destroy_device_notify(struct virtio_net *dev)
 	if (dev->flags & VIRTIO_DEV_RUNNING) {
 		vdpa_dev = dev->vdpa_dev;
-		if (vdpa_dev)
+		if (vdpa_dev && vdpa_dev->ops->dev_close)
 			vdpa_dev->ops->dev_close(dev->vid);
 		dev->flags &= ~VIRTIO_DEV_RUNNING;
-		dev->notify_ops->destroy_device(dev->vid);
+		if (dev->notify_ops->destroy_device)
+			dev->notify_ops->destroy_device(dev->vid);
 	}
 }
diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 52d8078d7c..26dc0bde97 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -3302,5 +3302,6 @@  vhost_user_msg_handler(int vid, int fd)
 
 	if (!(dev->flags & VIRTIO_DEV_RUNNING)) {
-		if (dev->notify_ops->new_device(dev->vid) == 0)
+		if (!dev->notify_ops->new_device ||
+			dev->notify_ops->new_device(dev->vid) == 0)
 			dev->flags |= VIRTIO_DEV_RUNNING;
 	}