Re: [Qemu-devel] [Bug 1529859] [NEW] qemu 2.5.0 ivshmem segfault with msi=off option

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1817 bytes --]

On 12/29/2015 06:38 AM, maquefel wrote:
> Public bug reported:
> 
> Launching qemu with "-device ivshmem,chardev=ivshmemid,msi=off -chardev
> socket,path=/tmp/ivshmem_socket,id=ivshmemid"
> 
> Causes segfault because, s->msi_vectors is not initialized and
> s->msi_vectors == 0.
> 
> Does ivshmem exactly need this line ? :
> 
> s->msi_vectors[vector].pdev = pdev;
> 
> It makes no sence for me.
> 
> Subject: [PATCH] fixed ivshmem empty msi vector on msi=off segfault

Patches require a Signed-off-by: line before they can be applied.

> 
> ---
>  hw/misc/ivshmem.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index f73f0c2..2087d5e 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -359,8 +359,6 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier *
>      int eventfd = event_notifier_get_fd(n);
>      CharDriverState *chr;
>  
> -    s->msi_vectors[vector].pdev = pdev;
> -

This avoids the segfault, but it may break other uses. Are you sure you
don't need an 'if (s->msi_vectors[vector])' conditional?

>      chr = qemu_chr_open_eventfd(eventfd);
>  
>      if (chr == NULL) {
> @@ -1038,10 +1036,11 @@ static void pci_ivshmem_exit(PCIDevice *dev)
>      }
>  
>      if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
> -        msix_uninit_exclusive_bar(dev);
> +        msix_uninit_exclusive_bar(dev);

I can't see what's changing here.  Whitespace?

>      }
> -
> -    g_free(s->msi_vectors);
> +    
> +    if(s->msi_vectors)
> +       g_free(s->msi_vectors);

This hunk is bogus.  g_free(NULL) already works properly.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]