[v2,1/2] virtio: Allow .get_vhost() without vhost_started

Commit Message

Hanna Czenczek July 23, 2024, 4:39 p.m. UTC

Historically, .get_vhost() was probably only called when
vdev->vhost_started is true.  However, we now decidedly want to call it
also when vhost_started is false, specifically so we can issue a reset
to the vhost back-end while device operation is stopped.

Some .get_vhost() implementations dereference some pointers (or return
offsets from them) that are probably guaranteed to be non-NULL when
vhost_started is true, but not necessarily otherwise.  This patch makes
all such implementations check all such pointers, returning NULL if any
is NULL.

Signed-off-by: Hanna Czenczek <[e-mail]>
---
 include/hw/virtio/virtio.h  |  1 +
 hw/display/vhost-user-gpu.c |  2 +-
 hw/net/virtio-net.c         | 19 +++++++++++++++++--
 hw/virtio/virtio-crypto.c   | 18 +++++++++++++++---
 4 files changed, 34 insertions(+), 6 deletions(-)

Comments

Stefan Hajnoczi July 30, 2024, 4:53 p.m. UTC | #1

On Tue, Jul 23, 2024 at 06:39:39PM +0200, Hanna Czenczek wrote:
> Historically, .get_vhost() was probably only called when
> vdev->vhost_started is true.  However, we now decidedly want to call it
> also when vhost_started is false, specifically so we can issue a reset
> to the vhost back-end while device operation is stopped.
> 
> Some .get_vhost() implementations dereference some pointers (or return
> offsets from them) that are probably guaranteed to be non-NULL when
> vhost_started is true, but not necessarily otherwise.  This patch makes
> all such implementations check all such pointers, returning NULL if any
> is NULL.
> 
> Signed-off-by: Hanna Czenczek <[e-mail]>
> ---
>  include/hw/virtio/virtio.h  |  1 +
>  hw/display/vhost-user-gpu.c |  2 +-
>  hw/net/virtio-net.c         | 19 +++++++++++++++++--
>  hw/virtio/virtio-crypto.c   | 18 +++++++++++++++---
>  4 files changed, 34 insertions(+), 6 deletions(-)

Reviewed-by: Stefan Hajnoczi <[e-mail]>

Patch

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 7512afbc84..ff29088232 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -221,6 +221,7 @@  struct VirtioDeviceClass {
     int (*post_load)(VirtIODevice *vdev);
     const VMStateDescription *vmsd;
     bool (*primary_unplug_pending)(void *opaque);
+    /* May be called even when vdev->vhost_started is false */
     struct vhost_dev *(*get_vhost)(VirtIODevice *vdev);
     void (*toggle_device_iotlb)(VirtIODevice *vdev);
 };
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 63c64ddde6..5056ffd618 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -642,7 +642,7 @@  vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp)
 static struct vhost_dev *vhost_user_gpu_get_vhost(VirtIODevice *vdev)
 {
     VhostUserGPU *g = VHOST_USER_GPU(vdev);
-    return &g->vhost->dev;
+    return g->vhost ? &g->vhost->dev : NULL;
 }
 
 static Property vhost_user_gpu_properties[] = {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 8f30972708..874ed9c032 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3890,8 +3890,23 @@  static bool dev_unplug_pending(void *opaque)
 static struct vhost_dev *virtio_net_get_vhost(VirtIODevice *vdev)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
-    NetClientState *nc = qemu_get_queue(n->nic);
-    struct vhost_net *net = get_vhost_net(nc->peer);
+    NetClientState *nc;
+    struct vhost_net *net;
+
+    if (!n->nic) {
+        return NULL;
+    }
+
+    nc = qemu_get_queue(n->nic);
+    if (!nc) {
+        return NULL;
+    }
+
+    net = get_vhost_net(nc->peer);
+    if (!net) {
+        return NULL;
+    }
+
     return &net->dev;
 }
 
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index bbe8aa4b99..3296b7f219 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -1247,9 +1247,21 @@  static bool virtio_crypto_guest_notifier_pending(VirtIODevice *vdev, int idx)
 static struct vhost_dev *virtio_crypto_get_vhost(VirtIODevice *vdev)
 {
     VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev);
-    CryptoDevBackend *b = vcrypto->cryptodev;
-    CryptoDevBackendClient *cc = b->conf.peers.ccs[0];
-    CryptoDevBackendVhost *vhost_crypto = cryptodev_get_vhost(cc, b, 0);
+    CryptoDevBackend *b;
+    CryptoDevBackendClient *cc;
+    CryptoDevBackendVhost *vhost_crypto;
+
+    b = vcrypto->cryptodev;
+    if (!b) {
+        return NULL;
+    }
+
+    cc = b->conf.peers.ccs[0];
+    vhost_crypto = cryptodev_get_vhost(cc, b, 0);
+    if (!vhost_crypto) {
+        return NULL;
+    }
+
     return &vhost_crypto->dev;
 }