| Message ID | 20151229133835.25580.50152.malonedeb@soybean.canonical.com |
|---|---|
| State | New |
| Headers | show |
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.
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 <eblake@redhat.com> 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 >
>See previously posted patch: > http://lists.gnu.org/archive/html/qemu-stable/2015-12/msg00034.html This patch resolves issue. I can confirm it works. ** Changed in: qemu Status: New => Fix Committed
Patch has been included here:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=47213eb1104709bf23
** Changed in: qemu
Status: Fix Committed => Fix Released
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; - 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); } - - g_free(s->msi_vectors); + + if(s->msi_vectors) + g_free(s->msi_vectors); } static bool test_msix(void *opaque, int version_id)