Marek Marczykowski
2011-Jun-09 23:33 UTC
[Xen-devel] [PATCH] libxl: do not try to detach pci device when none attached
# HG changeset patch # User Marek Marczykowski <marmarek@mimuw.edu.pl> # Date 1307662302 -7200 # Node ID 88f67b423c89f4bd604837b9eae2483dad5ebb0d # Parent 4b392511ae0840fba66c40aa2788dc1ff402b6e8 libxl: do not try to detach pci device when none attached Fixes SEGV on domain destroy Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -1039,6 +1039,8 @@ int libxl_device_pci_shutdown(libxl_ctx rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num); if ( rc ) return rc; + if ( num == 0 ) + return 0; for (i = 0; i < num; i++) { /* Force remove on shutdown since, on HVM, qemu will not always * respond to SCI interrupt because the guest kernel has shut down the _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jun-10 10:44 UTC
Re: [Xen-devel] [PATCH] libxl: do not try to detach pci device when none attached
On Fri, 10 Jun 2011, Marek Marczykowski wrote:> # HG changeset patch > # User Marek Marczykowski <marmarek@mimuw.edu.pl> > # Date 1307662302 -7200 > # Node ID 88f67b423c89f4bd604837b9eae2483dad5ebb0d > # Parent 4b392511ae0840fba66c40aa2788dc1ff402b6e8 > libxl: do not try to detach pci device when none attached > > Fixes SEGV on domain destroy > > Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> > > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c > --- a/tools/libxl/libxl_pci.c > +++ b/tools/libxl/libxl_pci.c > @@ -1039,6 +1039,8 @@ int libxl_device_pci_shutdown(libxl_ctx > rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num); > if ( rc ) > return rc; > + if ( num == 0 ) > + return 0; > for (i = 0; i < num; i++) { > /* Force remove on shutdown since, on HVM, qemu will not always > * respond to SCI interrupt because the guest kernel has shut down theIs the problem here that num is 0 and pcidevs is uninitialized? If so, would this patch solve the problem you are seeing? diff -r 37c77bacb52a tools/libxl/libxl_pci.c --- a/tools/libxl/libxl_pci.c Mon May 23 17:38:28 2011 +0100 +++ b/tools/libxl/libxl_pci.c Fri Jun 10 10:39:05 2011 +0000 @@ -1022,8 +1022,7 @@ int libxl_device_pci_list_assigned(libxl } while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL); } } - if ( *num ) - *list = pcidevs; + *list = pcidevs; libxl__free_all(&gc); return 0; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-10 10:48 UTC
Re: [Xen-devel] [PATCH] libxl: do not try to detach pci device when none attached
On 10.06.2011 12:44, Stefano Stabellini wrote:> On Fri, 10 Jun 2011, Marek Marczykowski wrote: >> # HG changeset patch >> # User Marek Marczykowski <marmarek@mimuw.edu.pl> >> # Date 1307662302 -7200 >> # Node ID 88f67b423c89f4bd604837b9eae2483dad5ebb0d >> # Parent 4b392511ae0840fba66c40aa2788dc1ff402b6e8 >> libxl: do not try to detach pci device when none attached >> >> Fixes SEGV on domain destroy >> >> Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> >> >> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c >> --- a/tools/libxl/libxl_pci.c >> +++ b/tools/libxl/libxl_pci.c >> @@ -1039,6 +1039,8 @@ int libxl_device_pci_shutdown(libxl_ctx >> rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num); >> if ( rc ) >> return rc; >> + if ( num == 0 ) >> + return 0; >> for (i = 0; i < num; i++) { >> /* Force remove on shutdown since, on HVM, qemu will not always >> * respond to SCI interrupt because the guest kernel has shut down the > > Is the problem here that num is 0 and pcidevs is uninitialized? > If so, would this patch solve the problem you are seeing?No, when there is no PCI devs, libxl_device_pci_list_assigned exists earlier - before allocating memory for pcidevs.> diff -r 37c77bacb52a tools/libxl/libxl_pci.c > --- a/tools/libxl/libxl_pci.c Mon May 23 17:38:28 2011 +0100 > +++ b/tools/libxl/libxl_pci.c Fri Jun 10 10:39:05 2011 +0000 > @@ -1022,8 +1022,7 @@ int libxl_device_pci_list_assigned(libxl > } while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL); > } > } > - if ( *num ) > - *list = pcidevs; > + *list = pcidevs; > libxl__free_all(&gc); > return 0; > }-- Pozdrawiam / Best Regards, Marek Marczykowski | RLU #390519 marmarek at mimuw edu pl | xmpp:marmarek at staszic waw pl _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jun-10 11:03 UTC
Re: [Xen-devel] [PATCH] libxl: do not try to detach pci device when none attached
On Fri, 10 Jun 2011, Marek Marczykowski wrote:> On 10.06.2011 12:44, Stefano Stabellini wrote: > > On Fri, 10 Jun 2011, Marek Marczykowski wrote: > >> # HG changeset patch > >> # User Marek Marczykowski <marmarek@mimuw.edu.pl> > >> # Date 1307662302 -7200 > >> # Node ID 88f67b423c89f4bd604837b9eae2483dad5ebb0d > >> # Parent 4b392511ae0840fba66c40aa2788dc1ff402b6e8 > >> libxl: do not try to detach pci device when none attached > >> > >> Fixes SEGV on domain destroy > >> > >> Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> > >> > >> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c > >> --- a/tools/libxl/libxl_pci.c > >> +++ b/tools/libxl/libxl_pci.c > >> @@ -1039,6 +1039,8 @@ int libxl_device_pci_shutdown(libxl_ctx > >> rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num); > >> if ( rc ) > >> return rc; > >> + if ( num == 0 ) > >> + return 0; > >> for (i = 0; i < num; i++) { > >> /* Force remove on shutdown since, on HVM, qemu will not always > >> * respond to SCI interrupt because the guest kernel has shut down the > > > > Is the problem here that num is 0 and pcidevs is uninitialized? > > If so, would this patch solve the problem you are seeing? > > No, when there is no PCI devs, libxl_device_pci_list_assigned exists > earlier - before allocating memory for pcidevs.Sorry, but I still don''t understand. If num is 0, it won''t enter the following loop anyway, the only other operation in libxl_device_pci_shutdown is free(pcidevs) and pcidevs should be NULL so nothing bad should happen. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Marek Marczykowski
2011-Jun-10 11:14 UTC
Re: [Xen-devel] [PATCH] libxl: do not try to detach pci device when none attached
On 10.06.2011 13:03, Stefano Stabellini wrote:> On Fri, 10 Jun 2011, Marek Marczykowski wrote: >> On 10.06.2011 12:44, Stefano Stabellini wrote: >>> On Fri, 10 Jun 2011, Marek Marczykowski wrote: >>>> # HG changeset patch >>>> # User Marek Marczykowski <marmarek@mimuw.edu.pl> >>>> # Date 1307662302 -7200 >>>> # Node ID 88f67b423c89f4bd604837b9eae2483dad5ebb0d >>>> # Parent 4b392511ae0840fba66c40aa2788dc1ff402b6e8 >>>> libxl: do not try to detach pci device when none attached >>>> >>>> Fixes SEGV on domain destroy >>>> >>>> Signed-off-by: Marek Marczykowski <marmarek@mimuw.edu.pl> >>>> >>>> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c >>>> --- a/tools/libxl/libxl_pci.c >>>> +++ b/tools/libxl/libxl_pci.c >>>> @@ -1039,6 +1039,8 @@ int libxl_device_pci_shutdown(libxl_ctx >>>> rc = libxl_device_pci_list_assigned(ctx, &pcidevs, domid, &num); >>>> if ( rc ) >>>> return rc; >>>> + if ( num == 0 ) >>>> + return 0; >>>> for (i = 0; i < num; i++) { >>>> /* Force remove on shutdown since, on HVM, qemu will not always >>>> * respond to SCI interrupt because the guest kernel has shut down the >>> >>> Is the problem here that num is 0 and pcidevs is uninitialized? >>> If so, would this patch solve the problem you are seeing? >> >> No, when there is no PCI devs, libxl_device_pci_list_assigned exists >> earlier - before allocating memory for pcidevs. > > Sorry, but I still don''t understand. > > If num is 0, it won''t enter the following loop anyway, the only > other operation in libxl_device_pci_shutdown is free(pcidevs) and > pcidevs should be NULL so nothing bad should happen.Ah, ok. So your patch should also works. -- Pozdrawiam / Best Regards, Marek Marczykowski | RLU #390519 marmarek at mimuw edu pl | xmpp:marmarek at staszic waw pl _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-21 16:34 UTC
Re: [Xen-devel] [PATCH] libxl: do not try to detach pci device when none attached
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] libxl: do not try to detach pci device when none attached"):> diff -r 37c77bacb52a tools/libxl/libxl_pci.c > --- a/tools/libxl/libxl_pci.c Mon May 23 17:38:28 2011 +0100 > +++ b/tools/libxl/libxl_pci.c Fri Jun 10 10:39:05 2011 +0000 > @@ -1022,8 +1022,7 @@ int libxl_device_pci_list_assigned(libxl > } while ((p = strtok_r(NULL, ",=", &saveptr)) != NULL); > } > } > - if ( *num ) > - *list = pcidevs; > + *list = pcidevs;This patch looks correct to me, as far as it goes. If n==0 then calloc is passed 0 and may return 0 willy-nilly but since there''s no error check this doesn''t cause other trouble :-/. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jun-21 16:36 UTC
Re: [Xen-devel] [PATCH] libxl: do not try to detach pci device when none attached
Marek Marczykowski writes ("Re: [Xen-devel] [PATCH] libxl: do not try to detach pci device when none attached"):> On 10.06.2011 13:03, Stefano Stabellini wrote: > > If num is 0, it won''t enter the following loop anyway, the only > > other operation in libxl_device_pci_shutdown is free(pcidevs) and > > pcidevs should be NULL so nothing bad should happen. > > Ah, ok. So your patch should also works.I have applied Stefano''s patch. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel