[libvirt] [PATCH] qemu: avoid NULL derefs

[Daniel Veillard]

On Mon, Feb 14, 2011 at 04:59:39PM -0700, Eric Blake wrote:
> The processWatchdogEvent fix is real, although it can only trigger on
> OOM, since bad things happen if doCoreDump is called with a NULL
> pathname argument.  The other fixes silence clang, but aren't a real
> bug because virReportErrorHelper tolerates a NULL format string even
> though *printf does not.
> 
> * src/qemu/qemu_driver.c (processWatchdogEvent): Exit on OOM.
> (qemuDomainIsActive, qemuDomainIsPersistent, qemuDomainIsUpdated):
> Provide valid message.
> ---
> 
> Another valid clang finding.  Maybe we whould mark
> virReportErrorHelper as ATTRIBUTE_NONNULL for the format string
> argument, and require all clients to pass in a valid error string
> (after all, that's what the printf attritube already assumes, and
> passing NULL just means that we aren't giving back any useful error
> information to the user); but that can be a separate cleanup.
> 
>  src/qemu/qemu_driver.c |   23 +++++++++++++++++------
>  1 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c581cfe..82a2210 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3489,7 +3489,10 @@ static int qemuDomainIsActive(virDomainPtr dom)
>      obj = virDomainFindByUUID(&driver->domains, dom->uuid);
>      qemuDriverUnlock(driver);
>      if (!obj) {
> -        qemuReportError(VIR_ERR_NO_DOMAIN, NULL);
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(dom->uuid, uuidstr);
> +        qemuReportError(VIR_ERR_NO_DOMAIN,
> +                        _("no domain with matching uuid '%s'"), uuidstr);
>          goto cleanup;
>      }
>      ret = virDomainObjIsActive(obj);
> @@ -3510,7 +3513,10 @@ static int qemuDomainIsPersistent(virDomainPtr dom)
>      obj = virDomainFindByUUID(&driver->domains, dom->uuid);
>      qemuDriverUnlock(driver);
>      if (!obj) {
> -        qemuReportError(VIR_ERR_NO_DOMAIN, NULL);
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(dom->uuid, uuidstr);
> +        qemuReportError(VIR_ERR_NO_DOMAIN,
> +                        _("no domain with matching uuid '%s'"), uuidstr);
>          goto cleanup;
>      }
>      ret = obj->persistent;
> @@ -3531,7 +3537,10 @@ static int qemuDomainIsUpdated(virDomainPtr dom)
>      obj = virDomainFindByUUID(&driver->domains, dom->uuid);
>      qemuDriverUnlock(driver);
>      if (!obj) {
> -        qemuReportError(VIR_ERR_NO_DOMAIN, NULL);
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(dom->uuid, uuidstr);
> +        qemuReportError(VIR_ERR_NO_DOMAIN,
> +                        _("no domain with matching uuid '%s'"), uuidstr);
>          goto cleanup;
>      }
>      ret = obj->updated;
> @@ -4981,12 +4990,14 @@ static void processWatchdogEvent(void *data, void *opaque)
>      case VIR_DOMAIN_WATCHDOG_ACTION_DUMP:
>          {
>              char *dumpfile;
> -            int i;
> 
> -            i = virAsprintf(&dumpfile, "%s/%s-%u",
> +            if (virAsprintf(&dumpfile, "%s/%s-%u",
>                              driver->autoDumpPath,
>                              wdEvent->vm->def->name,
> -                            (unsigned int)time(NULL));
> +                            (unsigned int)time(NULL)) < 0) {
> +                virReportOOMError();
> +                break;
> +            }
> 
>              qemuDriverLock(driver);
>              virDomainObjLock(wdEvent->vm);

  ACK, fine pushing now,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/