[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/7] vbox: avoid problematic uses of sprintf



2010/9/1 Eric Blake <eblake redhat com>:
> * src/vbox/vbox_tmpl.c (vboxStartMachine, vboxAttachUSB): Use
> virAsprintf instead.
> ---
>
> This removes all use of sprintf in vbox.  The first 3 use virAsprintf
> (DISPLAY may be arbitrarily long, and while we are unlikely to hit
> 9999 devices, it's better to be safe than to risk silent buffer
> overflow); the remaining two are sized appropriately (actually, they
> are sized too large, the real boundary size would be sizeof(int)*2+1
> rather than 40); I felt better using snprintf rather than sprintf.
>
> This doesn't address the fact that vbox doesn't really have very good
> OOM handling (ie. it keeps on trying, although after the first OOM,
> it will likely get another one); but that is an independent issue.
>


> @@ -4457,15 +4466,19 @@ vboxAttachUSB(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
>                     if (def->hostdevs[i]->source.subsys.type ==
>                         VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
>
> -                        char filtername[11]        = {0};
> +                        char *filtername           = NULL;
>                         PRUnichar *filternameUtf16 = NULL;
>                         IUSBDeviceFilter *filter   = NULL;
>
> -                        /* Assuming can't have more then 9999 devices so
> -                         * restricting to %04d
> +                        /* Zero pad for nice alignment when fewer than 9999
> +                         * devices.
>                          */
> -                        sprintf(filtername, "filter%04d", i);
> -                        VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
> +                        if (virAsprintf(&filtername, "filter%04d", i) < 0) {
> +                            virReportOOMError();
> +                        } else {
> +                            VBOX_UTF8_TO_UTF16(filtername, &filternameUtf16);
> +                            VIR_FREE(filtername);
> +                        }
>
>                         USBController->vtbl->CreateDeviceFilter(USBController,
>                                                                 filternameUtf16,

In case virAsprintf fails CreateDeviceFilter will be called with
filternameUtf16 = NULL, that might trigger a segfault.

Matthias


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]