[Qemu-devel] [PATCH for-2.6] ivshmem: fix crash with ivshmem-plain

[Qemu-devel] [PATCH for-2.6] ivshmem: fix crash with ivshmem-plain

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

"qemu -device ivshmem-plain" will crash, because it doesn't check that
'memdev' was provided.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/misc/ivshmem.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 08bf5c6..fae4d08 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -868,7 +868,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
 
         s->ivshmem_bar2 = host_memory_backend_get_memory(s->hostmem,
                                                          &error_abort);
-    } else {
+    } else if (s->server_chr) {
         IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n",
                         s->server_chr->filename);
 
@@ -899,6 +899,8 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
             error_setg(errp, "failed to initialize interrupts");
             return;
         }
+    } else {
+        g_assert_not_reached();
     }
 
     vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
@@ -1048,10 +1050,25 @@ static void ivshmem_plain_init(Object *obj)
                              &error_abort);
 }
 
+static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
+{
+    IVShmemState *s = IVSHMEM_COMMON(dev);
+
+    if (!s->hostmem) {
+        error_setg(errp, "You must specify a 'memdev'");
+        return;
+    }
+
+    ivshmem_common_realize(dev, errp);
+}
+
+
 static void ivshmem_plain_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+    k->realize = ivshmem_plain_realize;
     dc->props = ivshmem_plain_properties;
     dc->vmsd = &ivshmem_plain_vmsd;
 }
-- 
2.5.5

Re: [Qemu-devel] [PATCH for-2.6] ivshmem: fix crash with ivshmem-plain

[Markus Armbruster]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> "qemu -device ivshmem-plain" will crash, because it doesn't check that
> 'memdev' was provided.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Oww!

> ---
>  hw/misc/ivshmem.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 08bf5c6..fae4d08 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -868,7 +868,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
>  
>          s->ivshmem_bar2 = host_memory_backend_get_memory(s->hostmem,
>                                                           &error_abort);
> -    } else {
> +    } else if (s->server_chr) {

I'd simply stick

           assert(s->server_chr)

here.

>          IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n",
>                          s->server_chr->filename);
>  
> @@ -899,6 +899,8 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
>              error_setg(errp, "failed to initialize interrupts");
>              return;
>          }
> +    } else {
> +        g_assert_not_reached();
>      }
>  
>      vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
> @@ -1048,10 +1050,25 @@ static void ivshmem_plain_init(Object *obj)
>                               &error_abort);
>  }
>  
> +static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
> +{
> +    IVShmemState *s = IVSHMEM_COMMON(dev);
> +
> +    if (!s->hostmem) {
> +        error_setg(errp, "You must specify a 'memdev'");
> +        return;
> +    }
> +
> +    ivshmem_common_realize(dev, errp);
> +}
> +
> +
>  static void ivshmem_plain_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> +    k->realize = ivshmem_plain_realize;
>      dc->props = ivshmem_plain_properties;
>      dc->vmsd = &ivshmem_plain_vmsd;
>  }

Same bug in ivshmem-doorbell.  Care to fix that, too?