[Libguestfs] [PATCH] daemon: initialize memory when handling DeviceList params

Pino Toscano ptoscano at redhat.com
Fri Aug 8 12:07:41 UTC 2014


When dealing with DeviceList parameters, the generator produces code
similar to the following:

  CLEANUP_FREE_STRING_LIST char **devices = NULL;
  [...]
  devices = malloc (sizeof (char *) * (args.devices.devices_len+1));
  {
    size_t i;
    for (i = 0; i < args.devices.devices_len; ++i)
      RESOLVE_DEVICE (args.devices.devices_val[i], devices[i],
                      , goto done);
    devices[i] = NULL;
  }

The block hidden within the RESOLVE_DEVICE macro is supposed to
assign something to devices[i]; on the other hand, the code in
RESOLVE_DEVICE can cause to just end (with an error) the current RPC,
which would cause the cleanup of the "devices" array... whose members
from the i-th to the (args.devices.devices_len-1)-th would be garbage
pointers, causing random memory to be free'd (and thus crashing the
daemon).

Avoid the access to garbage memory just by having a cleaned "devices"
array, so there will be always a NULL element after the initialized
members.

Add a test for vgcreate which passes a wrong device path causing the
situation above, to test that vgcreate would fail gracefully.
---
 generator/actions.ml | 11 ++++++++++-
 generator/daemon.ml  |  2 +-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/generator/actions.ml b/generator/actions.ml
index 48213c5..9570d9b 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -3929,7 +3929,16 @@ as C</dev/sda1>." };
          ["vgcreate"; "VG1"; "/dev/sda1 /dev/sda2"];
          ["vgcreate"; "VG2"; "/dev/sda3"];
          ["vgs"]],
-        "is_string_list (ret, 2, \"VG1\", \"VG2\")"), []
+        "is_string_list (ret, 2, \"VG1\", \"VG2\")"), [];
+      InitEmpty, Always, TestLastFail (
+        [["part_init"; "/dev/sda"; "mbr"];
+         ["part_add"; "/dev/sda"; "p"; "64"; "204799"];
+         ["part_add"; "/dev/sda"; "p"; "204800"; "409599"];
+         ["part_add"; "/dev/sda"; "p"; "409600"; "-64"];
+         ["pvcreate"; "/dev/sda1"];
+         ["pvcreate"; "/dev/sda2"];
+         ["pvcreate"; "/dev/sda3"];
+         ["vgcreate"; "VG1"; "/foo/bar /dev/sda2"]]), [];
     ];
     shortdesc = "create an LVM volume group";
     longdesc = "\
diff --git a/generator/daemon.ml b/generator/daemon.ml
index 951729c..06d49ef 100644
--- a/generator/daemon.ml
+++ b/generator/daemon.ml
@@ -360,7 +360,7 @@ cleanup_free_mountable (mountable_t *mountable)
             pr "  /* Copy the string list and apply device name translation\n";
             pr "   * to each one.\n";
             pr "   */\n";
-            pr "  %s = malloc (sizeof (char *) * (args.%s.%s_len+1));\n" n n n;
+            pr "  %s = calloc (args.%s.%s_len+1, sizeof (char *));\n" n n n;
             pr "  {\n";
             pr "    size_t i;\n";
             pr "    for (i = 0; i < args.%s.%s_len; ++i)\n" n n;
-- 
1.9.3




More information about the Libguestfs mailing list