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

[Laine Stump]

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