daemon: Define safe ADD_ARG macro for constructing arg lists on the stack.

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 755f47e..a20ee08 100644 (file)
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -28,6 +28,8 @@
 #include "actions.h"
 #include "optgroups.h"
 
+#define MAX_ARGS 64
+
 int
 optgroup_btrfs_available (void)
 {
@@ -41,13 +43,13 @@ do_btrfs_filesystem_resize (const char *filesystem, int64_t size)
   char *buf;
   char *err;
   int r;
-  const char *argv[16];
+  const char *argv[MAX_ARGS];
   size_t i = 0;
   char size_str[32];
 
-  argv[i++] = "btrfs";
-  argv[i++] = "filesystem";
-  argv[i++] = "resize";
+  ADD_ARG (argv, i, "btrfs");
+  ADD_ARG (argv, i, "filesystem");
+  ADD_ARG (argv, i, "resize");
 
   if (optargs_bitmask & GUESTFS_BTRFS_FILESYSTEM_RESIZE_SIZE_BITMASK) {
     if (size <= 0) {
@@ -56,10 +58,10 @@ do_btrfs_filesystem_resize (const char *filesystem, int64_t size)
     }
 
     snprintf (size_str, sizeof size_str, "%" PRIi64, size);
-    argv[i++] = size_str;
+    ADD_ARG (argv, i, size_str);
   }
   else
-    argv[i++] = "max";
+    ADD_ARG (argv, i, "max");
 
   buf = sysroot_path (filesystem);
   if (!buf) {
@@ -67,8 +69,8 @@ do_btrfs_filesystem_resize (const char *filesystem, int64_t size)
     return -1;
   }
 
-  argv[i++] = buf;
-  argv[i++] = NULL;
+  ADD_ARG (argv, i, buf);
+  ADD_ARG (argv, i, NULL);
 
   r = commandv (NULL, &err, argv);
   free (buf);

diff --git a/daemon/daemon.h b/daemon/daemon.h
index 6ed68fd..489c38d 100644 (file)
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -212,6 +212,18 @@ is_zero (const char *buffer, size_t size)
   return 1;
 }
 
+/* Helper for building up short lists of arguments.  Your code has to
+ * define MAX_ARGS to a suitable value.
+ */
+#define ADD_ARG(argv,i,v)                                               \
+  do {                                                                  \
+    if ((i) >= MAX_ARGS) {                                              \
+      fprintf (stderr, "%s: %d: internal error: exceeded MAX_ARGS (%zu) when constructing the command line\n", __FILE__, __LINE__, (size_t) MAX_ARGS); \
+      abort ();                                                         \
+    }                                                                   \
+    (argv)[(i)++] = (v);                                                \
+  } while (0)
+
 /* Helper for functions that need a root filesystem mounted.
  * NB. Cannot be used for FileIn functions.
  */

diff --git a/daemon/luks.c b/daemon/luks.c
index bbe6328..02620ef 100644 (file)
--- a/daemon/luks.c
+++ b/daemon/luks.c
@@ -26,6 +26,8 @@
 #include "actions.h"
 #include "optgroups.h"
 
+#define MAX_ARGS 64
+
 int
 optgroup_luks_available (void)
 {
@@ -95,17 +97,17 @@ luks_open (const char *device, const char *key, const char *mapname,
   if (!tempfile)
     return -1;
 
-  const char *argv[16];
+  const char *argv[MAX_ARGS];
   size_t i = 0;
 
-  argv[i++] = "cryptsetup";
-  argv[i++] = "-d";
-  argv[i++] = tempfile;
-  if (readonly) argv[i++] = "--readonly";
-  argv[i++] = "luksOpen";
-  argv[i++] = device;
-  argv[i++] = mapname;
-  argv[i++] = NULL;
+  ADD_ARG (argv, i, "cryptsetup");
+  ADD_ARG (argv, i, "-d");
+  ADD_ARG (argv, i, tempfile);
+  if (readonly) ADD_ARG (argv, i, "--readonly");
+  ADD_ARG (argv, i, "luksOpen");
+  ADD_ARG (argv, i, device);
+  ADD_ARG (argv, i, mapname);
+  ADD_ARG (argv, i, NULL);
 
   char *err;
   int r = commandv (NULL, &err, (const char * const *) argv);
@@ -170,23 +172,23 @@ luks_format (const char *device, const char *key, int keyslot,
   if (!tempfile)
     return -1;
 
-  const char *argv[16];
+  const char *argv[MAX_ARGS];
   char keyslot_s[16];
   size_t i = 0;
 
-  argv[i++] = "cryptsetup";
-  argv[i++] = "-q";
+  ADD_ARG (argv, i, "cryptsetup");
+  ADD_ARG (argv, i, "-q");
   if (cipher) {
-    argv[i++] = "--cipher";
-    argv[i++] = cipher;
+    ADD_ARG (argv, i, "--cipher");
+    ADD_ARG (argv, i, cipher);
   }
-  argv[i++] = "--key-slot";
+  ADD_ARG (argv, i, "--key-slot");
   snprintf (keyslot_s, sizeof keyslot_s, "%d", keyslot);
-  argv[i++] = keyslot_s;
-  argv[i++] = "luksFormat";
-  argv[i++] = device;
-  argv[i++] = tempfile;
-  argv[i++] = NULL;
+  ADD_ARG (argv, i, keyslot_s);
+  ADD_ARG (argv, i, "luksFormat");
+  ADD_ARG (argv, i, device);
+  ADD_ARG (argv, i, tempfile);
+  ADD_ARG (argv, i, NULL);
 
   char *err;
   int r = commandv (NULL, &err, (const char * const *) argv);
@@ -232,21 +234,21 @@ do_luks_add_key (const char *device, const char *key, const char *newkey,
     return -1;
   }
 
-  const char *argv[16];
+  const char *argv[MAX_ARGS];
   char keyslot_s[16];
   size_t i = 0;
 
-  argv[i++] = "cryptsetup";
-  argv[i++] = "-q";
-  argv[i++] = "-d";
-  argv[i++] = keyfile;
-  argv[i++] = "--key-slot";
+  ADD_ARG (argv, i, "cryptsetup");
+  ADD_ARG (argv, i, "-q");
+  ADD_ARG (argv, i, "-d");
+  ADD_ARG (argv, i, keyfile);
+  ADD_ARG (argv, i, "--key-slot");
   snprintf (keyslot_s, sizeof keyslot_s, "%d", keyslot);
-  argv[i++] = keyslot_s;
-  argv[i++] = "luksAddKey";
-  argv[i++] = device;
-  argv[i++] = newkeyfile;
-  argv[i++] = NULL;
+  ADD_ARG (argv, i, keyslot_s);
+  ADD_ARG (argv, i, "luksAddKey");
+  ADD_ARG (argv, i, device);
+  ADD_ARG (argv, i, newkeyfile);
+  ADD_ARG (argv, i, NULL);
 
   char *err;
   int r = commandv (NULL, &err, (const char * const *) argv);
@@ -271,19 +273,19 @@ do_luks_kill_slot (const char *device, const char *key, int keyslot)
   if (!tempfile)
     return -1;
 
-  const char *argv[16];
+  const char *argv[MAX_ARGS];
   char keyslot_s[16];
   size_t i = 0;
 
-  argv[i++] = "cryptsetup";
-  argv[i++] = "-q";
-  argv[i++] = "-d";
-  argv[i++] = tempfile;
-  argv[i++] = "luksKillSlot";
-  argv[i++] = device;
+  ADD_ARG (argv, i, "cryptsetup");
+  ADD_ARG (argv, i, "-q");
+  ADD_ARG (argv, i, "-d");
+  ADD_ARG (argv, i, tempfile);
+  ADD_ARG (argv, i, "luksKillSlot");
+  ADD_ARG (argv, i, device);
   snprintf (keyslot_s, sizeof keyslot_s, "%d", keyslot);
-  argv[i++] = keyslot_s;
-  argv[i++] = NULL;
+  ADD_ARG (argv, i, keyslot_s);
+  ADD_ARG (argv, i, NULL);
 
   char *err;
   int r = commandv (NULL, &err, (const char * const *) argv);

diff --git a/daemon/mkfs.c b/daemon/mkfs.c
index d073c07..f3975dc 100644 (file)
--- a/daemon/mkfs.c
+++ b/daemon/mkfs.c
@@ -29,7 +29,7 @@
 #include "daemon.h"
 #include "actions.h"
 
-#define MAX_ARGS 16
+#define MAX_ARGS 64
 
 /* Takes optional arguments, consult optargs_bitmask. */
 int
@@ -53,47 +53,47 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize,
       STREQ (fstype, "ext4")) {
     if (e2prog (mke2fs) == -1)
       return -1;
-    argv[i++] = mke2fs;
+    ADD_ARG (argv, i, mke2fs);
   }
   else
-    argv[i++] = "mkfs";
+    ADD_ARG (argv, i, "mkfs");
 
-  argv[i++] = "-t";
-  argv[i++] = fstype;
+  ADD_ARG (argv, i, "-t");
+  ADD_ARG (argv, i, fstype);
 
   /* Force mke2fs to create a filesystem, even if it thinks it
    * shouldn't (RHBZ#690819).
    */
   if (STREQ (fstype, "ext2") || STREQ (fstype, "ext3") ||
       STREQ (fstype, "ext4"))
-    argv[i++] = "-F";
+    ADD_ARG (argv, i, "-F");
 
   /* mkfs.ntfs requires the -Q argument otherwise it writes zeroes
    * to every block and does bad block detection, neither of which
    * are useful behaviour for virtual devices.
    */
   if (STREQ (fstype, "ntfs"))
-    argv[i++] = "-Q";
+    ADD_ARG (argv, i, "-Q");
 
   /* mkfs.reiserfs produces annoying interactive prompts unless you
    * tell it to be quiet.
    */
   if (STREQ (fstype, "reiserfs"))
-    argv[i++] = "-f";
+    ADD_ARG (argv, i, "-f");
 
   /* Same for JFS. */
   if (STREQ (fstype, "jfs"))
-    argv[i++] = "-f";
+    ADD_ARG (argv, i, "-f");
 
   /* For GFS, GFS2, assume a single node. */
   if (STREQ (fstype, "gfs") || STREQ (fstype, "gfs2")) {
-    argv[i++] = "-p";
-    argv[i++] = "lock_nolock";
+    ADD_ARG (argv, i, "-p");
+    ADD_ARG (argv, i, "lock_nolock");
     /* The man page says this is default, but it doesn't seem to be: */
-    argv[i++] = "-j";
-    argv[i++] = "1";
+    ADD_ARG (argv, i, "-j");
+    ADD_ARG (argv, i, "1");
     /* Don't ask questions: */
-    argv[i++] = "-O";
+    ADD_ARG (argv, i, "-O");
   }
 
   /* Process blocksize parameter if set. */
@@ -121,26 +121,26 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize,
       }
 
       snprintf (blocksize_str, sizeof blocksize_str, "%d", sectors_per_cluster);
-      argv[i++] = "-s";
-      argv[i++] = blocksize_str;
+      ADD_ARG (argv, i, "-s");
+      ADD_ARG (argv, i, blocksize_str);
     }
     else if (STREQ (fstype, "ntfs")) {
       /* For NTFS map the blocksize into a cluster size. */
       snprintf (blocksize_str, sizeof blocksize_str, "%d", blocksize);
-      argv[i++] = "-c";
-      argv[i++] = blocksize_str;
+      ADD_ARG (argv, i, "-c");
+      ADD_ARG (argv, i, blocksize_str);
     }
     else {
       /* For all other filesystem types, try the -b option. */
       snprintf (blocksize_str, sizeof blocksize_str, "%d", blocksize);
-      argv[i++] = "-b";
-      argv[i++] = blocksize_str;
+      ADD_ARG (argv, i, "-b");
+      ADD_ARG (argv, i, blocksize_str);
     }
   }
 
   if (optargs_bitmask & GUESTFS_MKFS_OPTS_FEATURES_BITMASK) {
-     argv[i++] = "-O";
-     argv[i++] = features;
+    ADD_ARG (argv, i, "-O");
+    ADD_ARG (argv, i, features);
   }
 
   if (optargs_bitmask & GUESTFS_MKFS_OPTS_INODE_BITMASK) {
@@ -156,8 +156,8 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize,
     }
 
     snprintf (inode_str, sizeof inode_str, "%d", inode);
-    argv[i++] = "-I";
-    argv[i++] = inode_str;
+    ADD_ARG (argv, i, "-I");
+    ADD_ARG (argv, i, inode_str);
   }
 
   if (optargs_bitmask & GUESTFS_MKFS_OPTS_SECTORSIZE_BITMASK) {
@@ -172,15 +172,12 @@ do_mkfs_opts (const char *fstype, const char *device, int blocksize,
     }
 
     snprintf (sectorsize_str, sizeof sectorsize_str, "%d", sectorsize);
-    argv[i++] = "-S";
-    argv[i++] = sectorsize_str;
+    ADD_ARG (argv, i, "-S");
+    ADD_ARG (argv, i, sectorsize_str);
   }
 
-  argv[i++] = device;
-  argv[i++] = NULL;
-
-  if (i > MAX_ARGS)
-    abort ();
+  ADD_ARG (argv, i, device);
+  ADD_ARG (argv, i, NULL);
 
   r = commandv (NULL, &err, argv);
   if (r == -1) {

diff --git a/daemon/ntfs.c b/daemon/ntfs.c
index 92d2432..076e297 100644 (file)
--- a/daemon/ntfs.c
+++ b/daemon/ntfs.c
@@ -28,6 +28,8 @@
 #include "actions.h"
 #include "optgroups.h"
 
+#define MAX_ARGS 64
+
 int
 optgroup_ntfs3g_available (void)
 {
@@ -66,12 +68,12 @@ do_ntfsresize_opts (const char *device, int64_t size, int force)
 {
   char *err;
   int r;
-  const char *argv[16];
+  const char *argv[MAX_ARGS];
   size_t i = 0;
   char size_str[32];
 
-  argv[i++] = "ntfsresize";
-  argv[i++] = "-P";
+  ADD_ARG (argv, i, "ntfsresize");
+  ADD_ARG (argv, i, "-P");
 
   if (optargs_bitmask & GUESTFS_NTFSRESIZE_OPTS_SIZE_BITMASK) {
     if (size <= 0) {
@@ -80,15 +82,15 @@ do_ntfsresize_opts (const char *device, int64_t size, int force)
     }
 
     snprintf (size_str, sizeof size_str, "%" PRIi64, size);
-    argv[i++] = "--size";
-    argv[i++] = size_str;
+    ADD_ARG (argv, i, "--size");
+    ADD_ARG (argv, i, size_str);
   }
 
   if (optargs_bitmask & GUESTFS_NTFSRESIZE_OPTS_FORCE_BITMASK && force)
-    argv[i++] = "--force";
+    ADD_ARG (argv, i, "--force");
 
-  argv[i++] = device;
-  argv[i++] = NULL;
+  ADD_ARG (argv, i, device);
+  ADD_ARG (argv, i, NULL);
 
   r = commandv (NULL, &err, argv);
   if (r == -1) {