DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/1] vhost: NULL callback checks
@ 2025-01-24 14:14 Kevin Traynor
  2025-01-24 14:14 ` [PATCH 1/1] vhost: add " Kevin Traynor
  0 siblings, 1 reply; 2+ messages in thread
From: Kevin Traynor @ 2025-01-24 14:14 UTC (permalink / raw)
  To: dev; +Cc: maxime.coquelin, Kevin Traynor

Not a normal case, but noticed that setting some callbacks to
NULL in rte_vhost_driver_callback_register() causes a seg fault e.g. [0].

Added missing checks before calling. Alternative fix is to make some
callbacks mandatory and check during register - is that an API change? maybe,
so took simpler approach.

A bit hard to find fixed commit due to the time and amount of
development in vhost code.

[0]

3302│         if (!(dev->flags & VIRTIO_DEV_RUNNING)) {
3303├>                if (dev->notify_ops->new_device(dev->vid) == 0)
3304│                         dev->flags |= VIRTIO_DEV_RUNNING;
3305│         }

(gdb) frame 1
at ../lib/vhost/vhost_user.c:3303
(gdb) p dev->notify_ops->new_device
$3 = (int (*)(int)) 0x0

Kevin Traynor (1):
  vhost: add NULL callback checks

 lib/vhost/vduse.c      | 3 ++-
 lib/vhost/vhost.c      | 5 +++--
 lib/vhost/vhost_user.c | 3 ++-
 3 files changed, 7 insertions(+), 4 deletions(-)

-- 
2.48.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH 1/1] vhost: add NULL callback checks
  2025-01-24 14:14 [PATCH 0/1] vhost: NULL callback checks Kevin Traynor
@ 2025-01-24 14:14 ` Kevin Traynor
  0 siblings, 0 replies; 2+ messages in thread
From: Kevin Traynor @ 2025-01-24 14:14 UTC (permalink / raw)
  To: dev; +Cc: maxime.coquelin, Kevin Traynor, stable

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;
 	}
-- 
2.48.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-01-24 14:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-24 14:14 [PATCH 0/1] vhost: NULL callback checks Kevin Traynor
2025-01-24 14:14 ` [PATCH 1/1] vhost: add " Kevin Traynor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).