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

Ryota Ozaki ozaki.ryota at gmail.com
Wed Nov 20 04:24:35 UTC 2013


On Tue, Nov 19, 2013 at 11:29 PM, Laine Stump <laine at laine.org> wrote:
> 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 at gmail.com>
>> ---
>>  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.

Thanks Laine!

> 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:

Yes, I'm using the vbox driver for testing on Mac OS X. Actually
the fixed bug was found when I was working on API v4.3 support.

>
> 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).

Thank you for pointing out. I'm queuing the fixes and will send patches
after I finish API v4.3 support.

  ozaki-r




More information about the libvir-list mailing list