From edd502543adbdc2fa5dda0c015ea7c390bb39f64 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Thu, 10 Nov 2011 15:53:23 +0000 Subject: [PATCH] daemon: Define safe ADD_ARG macro for constructing arg lists on the stack. --- daemon/btrfs.c | 18 +++++++------ daemon/daemon.h | 12 +++++++++ daemon/luks.c | 82 +++++++++++++++++++++++++++++---------------------------- daemon/mkfs.c | 59 ++++++++++++++++++++--------------------- daemon/ntfs.c | 18 +++++++------ 5 files changed, 102 insertions(+), 87 deletions(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index 755f47e..a20ee08 100644 --- 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 --- 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 --- 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 --- 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 --- 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) { -- 1.8.3.1