[libvirt] [PATCH] Fix more OOM handling bugs

[Daniel Veillard]

On Thu, Sep 03, 2009 at 05:39:53PM +0100, Daniel P. Berrange wrote:
> * src/qemu_conf.c: Fix leak of values upon OOM
> * src/xend_internal.c: Fix missing check for OOM failure
> * tests/qemuargv2xmltest.c, tests/qemuxml2argvtest.c: Free
>   stateDir upon exit to avoid leak
> ---
>  src/qemu_conf.c          |   21 ++++++++++++++++++---
>  src/xend_internal.c      |    6 ++++++
>  tests/qemuargv2xmltest.c |    1 +
>  tests/qemuxml2argvtest.c |    1 +
>  4 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu_conf.c b/src/qemu_conf.c
> index 22f5edd..ad41104 100644
> --- a/src/qemu_conf.c
> +++ b/src/qemu_conf.c
> @@ -1722,6 +1722,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
>                  continue;
>              }
>  
> +            ADD_ARG_SPACE;
> +
>              if (idx < 0) {
>                  qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
>                                   _("unsupported disk type '%s'"), disk->dst);
> @@ -1773,7 +1775,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
>  
>              optstr = virBufferContentAndReset(&opt);
>  
> -            ADD_ARG_LIT("-drive");
> +            if ((qargv[qargc++] = strdup("-drive")) == NULL) {
> +                VIR_FREE(optstr);
> +                goto no_memory;
> +            }
>              ADD_ARG(optstr);
>          }
>      } else {
> @@ -1829,6 +1834,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
>  
>              net->vlan = i;
>  
> +            ADD_ARG_SPACE;
>              if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) &&
>                  qemuAssignNetNames(def, net) < 0)
>                  goto no_memory;
> @@ -1836,9 +1842,14 @@ int qemudBuildCommandLine(virConnectPtr conn,
>              if (qemuBuildNicStr(conn, net, NULL, ',', net->vlan, &nic) < 0)
>                  goto error;
>  
> -            ADD_ARG_LIT("-net");
> +            if ((qargv[qargc++] = strdup("-net")) == NULL) {
> +                VIR_FREE(nic);
> +                goto no_memory;
> +            }
>              ADD_ARG(nic);
>  
> +
> +            ADD_ARG_SPACE;
>              if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK ||
>                  net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
>                  int tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags);
> @@ -1862,7 +1873,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
>                  goto error;
>              }
>  
> -            ADD_ARG_LIT("-net");
> +            if ((qargv[qargc++] = strdup("-net")) == NULL) {
> +                VIR_FREE(host);
> +                goto no_memory;
> +            }
>              ADD_ARG(host);
>  
>              VIR_FREE(tapfd_name);
> @@ -2230,6 +2244,7 @@ static int qemuStringToArgvEnv(const char *args,
>              goto no_memory;
>          for (i = 0 ; i < envend ; i++) {
>              progenv[i] = arglist[i];
> +            arglist[i] = NULL;
>          }
>          progenv[i] = NULL;
>      }
> diff --git a/src/xend_internal.c b/src/xend_internal.c
> index 99847b0..20ddb89 100644
> --- a/src/xend_internal.c
> +++ b/src/xend_internal.c
> @@ -5212,6 +5212,9 @@ xenDaemonFormatSxprChr(virConnectPtr conn,
>          break;
>      }
>  
> +    if (virBufferError(buf))
> +        return -1;
> +
>      return 0;
>  }
>  
> @@ -5535,6 +5538,9 @@ xenDaemonFormatSxprSound(virConnectPtr conn,
>          virBufferVSprintf(buf, "%s%s", i ? "," : "", str);
>      }
>  
> +    if (virBufferError(buf))
> +        return -1;
> +
>      return 0;
>  }
>  
> diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
> index 7861520..4a92280 100644
> --- a/tests/qemuargv2xmltest.c
> +++ b/tests/qemuargv2xmltest.c
> @@ -223,6 +223,7 @@ mymain(int argc, char **argv)
>      DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat");
>      DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000");
>  
> +    free(driver.stateDir);
>      virCapabilitiesFree(driver.caps);
>  
>      return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 6f25e7d..2f91288 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -270,6 +270,7 @@ mymain(int argc, char **argv)
>      DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat");
>      DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000");
>  
> +    free(driver.stateDir);
>      virCapabilitiesFree(driver.caps);
>  
>      return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);

  ACK,

  but if ADD_ARG_LIT can't be made safe w.r.t OOM, maybe the macro
definition should be removed too.

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/