Re: [PATCH 4/6] uml: Plug memory leak on umlStartVMDaemon() error path

[Eric Blake <eblake@xxxxxxxxxx>]

On 11/29/2011 10:57 PM, ajia@xxxxxxxxxx wrote:
> From: Alex Jia <ajia@xxxxxxxxxx>
> 
> Detected by Coverity.  Leak introduced in commit 8866eed.
> 
> Signed-off-by: Alex Jia <ajia@xxxxxxxxxx>
> ---
>  src/uml/uml_driver.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index 073f362..fe6d5ba 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -1051,6 +1051,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
>          VIR_FREE(vm->def->consoles[i]->info.alias);
>          if (virAsprintf(&vm->def->consoles[i]->info.alias, "console%zu", i) < 0) {
>              virReportOOMError();
> +            VIR_FORCE_CLOSE(logfd);
>              goto cleanup;

Good catch, but while reading the function, I found another bug - if we
failed to add the domain to autodestroy, we still returned success but
without setting the domain to transient.  I'm pushing this more
comprehensive fix:

diff --git i/src/uml/uml_driver.c w/src/uml/uml_driver.c
index fe6d5ba..faea313 100644
--- i/src/uml/uml_driver.c
+++ w/src/uml/uml_driver.c
@@ -1033,56 +1033,51 @@ static int umlStartVMDaemon(virConnectPtr conn,
     /* Do this upfront, so any part of the startup process can add
      * runtime state to vm->def that won't be persisted. This let's us
      * report implicit runtime defaults in the XML, like vnc listen/socket
      */
     VIR_DEBUG("Setting current domain def as transient");
     if (virDomainObjSetDefTransient(driver->caps, vm, true) < 0) {
         VIR_FORCE_CLOSE(logfd);
         return -1;
     }

-    if (!(cmd = umlBuildCommandLine(conn, driver, vm))) {
-        VIR_FORCE_CLOSE(logfd);
-        virDomainConfVMNWFilterTeardown(vm);
-        umlCleanupTapDevices(vm);
-        return -1;
-    }
+    if (!(cmd = umlBuildCommandLine(conn, driver, vm)))
+        goto cleanup;

     for (i = 0 ; i < vm->def->nconsoles ; i++) {
         VIR_FREE(vm->def->consoles[i]->info.alias);
         if (virAsprintf(&vm->def->consoles[i]->info.alias,
"console%zu", i) < 0) {
             virReportOOMError();
-            VIR_FORCE_CLOSE(logfd);
             goto cleanup;
         }
     }

     virCommandWriteArgLog(cmd, logfd);

     priv->monitor = -1;

     virCommandClearCaps(cmd);
     virCommandSetOutputFD(cmd, &logfd);
     virCommandSetErrorFD(cmd, &logfd);
     virCommandDaemonize(cmd);

     ret = virCommandRun(cmd, NULL);
-    VIR_FORCE_CLOSE(logfd);
     if (ret < 0)
         goto cleanup;

     if (autoDestroy &&
-        umlProcessAutoDestroyAdd(driver, vm, conn) < 0)
+        (ret = umlProcessAutoDestroyAdd(driver, vm, conn)) < 0)
         goto cleanup;

     ret = virDomainObjSetDefTransient(driver->caps, vm, false);
 cleanup:
+    VIR_FORCE_CLOSE(logfd);
     virCommandFree(cmd);

     if (ret < 0) {
         virDomainConfVMNWFilterTeardown(vm);
         umlCleanupTapDevices(vm);
         if (vm->newDef) {
             virDomainDefFree(vm->def);
             vm->def = vm->newDef;
             vm->def->id = -1;
             vm->newDef = NULL;


-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list