qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Bug 1529859] [NEW] qemu 2.5.0 ivshmem segfault with ms


From: Marc-André Lureau
Subject: Re: [Qemu-devel] [Bug 1529859] [NEW] qemu 2.5.0 ivshmem segfault with msi=off option
Date: Mon, 4 Jan 2016 22:46:18 +0100

See previously posted patch:

http://lists.gnu.org/archive/html/qemu-stable/2015-12/msg00034.html

On Mon, Jan 4, 2016 at 9:24 PM, Eric Blake <address@hidden> wrote:
> 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
>



-- 
Marc-André Lureau



reply via email to

[Prev in Thread] Current Thread [Next in Thread]