[PATCH] Fix misc bugs in virCommandPtr

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

 



The virCommandNewArgs() method would free the virCommandPtr
if it failed to add the args. This meant errors reported in
virCommandAddArgSet() were lost. Simply removing the check
for errors from the constructor means they can be reported
correctly later

The virCommandAddEnvPassCommon() method failed to check for
errors before reallocating the cmd->env array, causing a
potential SEGV if cmd was NULL

The virCommandAddArgSet() method needs to validate that at
least 1 element in 'val's parameter is non-NULL, otherwise
code like

    cmd = virCommandNew(binary)
    virCommandAddAtg(cmd, "foo")

Would end up trying todo  execve("foo"), if binary was
NULL.
---
 src/util/command.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/util/command.c b/src/util/command.c
index ff2bd46..22f350b 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -109,11 +109,6 @@ virCommandNewArgs(const char *const*args)
 
     virCommandAddArgSet(cmd, args);
 
-    if (cmd->has_error) {
-        virCommandFree(cmd);
-        return NULL;
-    }
-
     return cmd;
 }
 
@@ -362,6 +357,9 @@ virCommandAddEnvPass(virCommandPtr cmd, const char *name)
 void
 virCommandAddEnvPassCommon(virCommandPtr cmd)
 {
+    if (!cmd || cmd->has_error)
+        return;
+
     /* Attempt to Pre-allocate; allocation failure will be detected
      * later during virCommandAdd*.  */
     ignore_value(VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 9));
@@ -478,6 +476,11 @@ virCommandAddArgSet(virCommandPtr cmd, const char *const*vals)
     if (!cmd || cmd->has_error)
         return;
 
+    if (vals[0] == NULL) {
+        cmd->has_error = EINVAL;
+        return;
+    }
+
     while (vals[narg] != NULL)
         narg++;
 
-- 
1.7.4

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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]