Re: [libvirt] [PATCH] vbox: fix segfault on virsh dumpxml with the existence of USB filters

On 11/18/2013 05:39 PM, Ryota Ozaki wrote: > A USB filter is stored in a hostdev. The original code doesn't > allocate hostdev->info that is expected to be allocated with hostdev. > So use virDomainHostdevDefAlloc() to allocate both as we expect. > > Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com&gt; > --- > src/vbox/vbox_tmpl.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c > index 3807a6d..f4cd5cf 100644 > --- a/src/vbox/vbox_tmpl.c > +++ b/src/vbox/vbox_tmpl.c > @@ -3333,7 +3333,8 @@ sharedFoldersCleanup: > > deviceFilter->vtbl->GetActive(deviceFilter, &active); > if (active) { > - if (VIR_ALLOC(def->hostdevs[USBFilterCount]) >= 0) { > + def->hostdevs[USBFilterCount] = virDomainHostdevDefAlloc(); > + if (def->hostdevs[USBFilterCount]) { > PRUnichar *vendorIdUtf16 = NULL; > char *vendorIdUtf8 = NULL; > unsigned vendorId = 0; ACK on this fix, which I pushed.

But just taking a quick look at that small chunk of the gigantic function it's part of (vboxDomainGetXMLDesc), I saw two other problems that need fixing. Since you seem to have an interest in a properly working vbox driver, you might want to look into making patches to:

1) properly handle out of memory errors (and other errors - there is absolutely no error handling in that function). 2) Fix this apparently incorrect comparison just above the current patch: for (i = 0; (USBFilterCount < def->nhostdevs) || (i < deviceFilters.count); i++) { (I'm pretty sure that "||" needs to be "&&", but don't want to patch code that I can't test).