>From fcc190a856a537bb1ca429714e54468ca3514f33 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Mon, 27 Jul 2009 22:27:45 +0100 Subject: [PATCH 1/2] Replace shell_quote function with %Q and %R printf specifiers. %Q => simple shell quoted string %R => path will be prefixed by /sysroot eg. snprintf (cmd, sizeof cmd, "cat %R", path); system (cmd); --- daemon/daemon.h | 17 ++++++++++++- daemon/file.c | 24 ++++++++----------- daemon/find.c | 11 +------- daemon/grub.c | 9 ++----- daemon/guestfsd.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++ daemon/initrd.c | 16 ++++--------- daemon/tar.c | 60 +++++++++++++++++++----------------------------- 7 files changed, 124 insertions(+), 78 deletions(-) diff --git a/daemon/daemon.h b/daemon/daemon.h index c2bbf3e..2ab1574 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -54,12 +54,25 @@ extern int commandrv (char **stdoutput, char **stderror, extern char **split_lines (char *str); -extern int shell_quote (char *out, int len, const char *in); - extern int device_name_translation (char *device, const char *func); extern void udev_settle (void); +/* This just stops gcc from giving a warning about our custom + * printf formatters %Q and %R. + */ +static inline int +asprintf_nowarn (char **strp, const char *fmt, ...) +{ + int r; + va_list args; + + va_start (args, fmt); + r = vasprintf (strp, fmt, args); + va_end (args); + return r; +} + /*-- in names.c (auto-generated) --*/ extern const char *function_names[]; diff --git a/daemon/file.c b/daemon/file.c index 7c46722..6062c50 100644 --- a/daemon/file.c +++ b/daemon/file.c @@ -440,6 +440,7 @@ char * do_zfile (char *method, char *path) { int len; + const char *zcat; char *cmd; FILE *fp; char line[256]; @@ -447,27 +448,22 @@ do_zfile (char *method, char *path) NEED_ROOT (NULL); ABS_PATH (path, NULL); - len = 2 * strlen (path) + sysroot_len + 64; - cmd = malloc (len); - if (!cmd) { - reply_with_perror ("malloc"); - return NULL; - } - if (strcmp (method, "gzip") == 0 || strcmp (method, "compress") == 0) - strcpy (cmd, "zcat"); + zcat = "zcat"; else if (strcmp (method, "bzip2") == 0) - strcpy (cmd, "bzcat"); + zcat = "bzcat"; else { - free (cmd); reply_with_error ("zfile: unknown method"); return NULL; } - strcat (cmd, " "); - strcat (cmd, sysroot); - shell_quote (cmd + strlen (cmd), len - strlen (cmd), path); - strcat (cmd, " | file -bsL -"); + if (asprintf_nowarn (&cmd, "%s %R | file -bsL -", zcat, path) == -1) { + reply_with_perror ("asprintf"); + return NULL; + } + + if (verbose) + fprintf (stderr, "%s\n", cmd); fp = popen (cmd, "r"); if (fp == NULL) { diff --git a/daemon/find.c b/daemon/find.c index d882953..40f1b3b 100644 --- a/daemon/find.c +++ b/daemon/find.c @@ -83,21 +83,14 @@ do_find (char *dir) sysrootdirlen = strlen (sysrootdir); /* Assemble the external find command. */ - len = 2 * sysrootdirlen + 32; - cmd = malloc (len); - if (!cmd) { + if (asprintf_nowarn (&cmd, "find %Q -print0", sysrootdir) == -1) { reply_with_perror ("malloc"); free (sysrootdir); return NULL; } - strcpy (cmd, "find "); - shell_quote (cmd+5, len-5, sysrootdir); - free (sysrootdir); - strcat (cmd, " -print0"); - if (verbose) - printf ("%s\n", cmd); + fprintf (stderr, "%s\n", cmd); fp = popen (cmd, "r"); if (fp == NULL) { diff --git a/daemon/grub.c b/daemon/grub.c index 3b1768f..86e812e 100644 --- a/daemon/grub.c +++ b/daemon/grub.c @@ -28,7 +28,7 @@ int do_grub_install (char *root, char *device) { - int r, len; + int r; char *err; char *buf; @@ -36,13 +36,10 @@ do_grub_install (char *root, char *device) ABS_PATH (root, -1); IS_DEVICE (device, -1); - len = strlen (root) + sysroot_len + 64; - buf = malloc (len); - if (!buf) { - reply_with_perror ("malloc"); + if (asprintf_nowarn (&buf, "--root-directory=%R", root) == -1) { + reply_with_perror ("asprintf"); return -1; } - snprintf (buf, len, "--root-directory=%s%s", sysroot, root); r = command (NULL, &err, "/sbin/grub-install", buf, device, NULL); free (buf); diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 5c250f0..6c044b6 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -36,6 +36,7 @@ #include #include #include +#include #include "daemon.h" @@ -47,6 +48,10 @@ static void usage (void); int verbose = 0; +static int print_shell_quote (FILE *stream, const struct printf_info *info, const void *const *args); +static int print_sysroot_shell_quote (FILE *stream, const struct printf_info *info, const void *const *args); +static int print_arginfo (const struct printf_info *info, size_t n, int *argtypes, int *size); + /* Location to mount root device. */ const char *sysroot = "/sysroot"; /* No trailing slash. */ int sysroot_len = 8; @@ -76,6 +81,10 @@ main (int argc, char *argv[]) uint32_t len; struct sigaction sa; + /* http://udrepper.livejournal.com/20948.html */ + register_printf_specifier ('Q', print_shell_quote, print_arginfo); + register_printf_specifier ('R', print_sysroot_shell_quote, print_arginfo); + for (;;) { c = getopt_long (argc, argv, options, long_options, NULL); if (c == -1) break; @@ -229,6 +238,8 @@ main (int argc, char *argv[]) * * Caller must check for NULL and call reply_with_perror ("malloc") * if it is. Caller must also free the string. + * + * See also the custom %R printf formatter which does shell quoting too. */ char * sysroot_path (const char *path) @@ -685,6 +696,59 @@ split_lines (char *str) return lines; } +/* printf helper function so we can use %Q ("quoted") and %R to print + * shell-quoted strings. + * + * %Q => simple shell quoted string + * %R => path will be prefixed by /sysroot + * + * eg. snprintf (cmd, sizeof cmd, "cat %R", path); system (cmd); + */ +static int +print_shell_quote (FILE *stream, + const struct printf_info *info, + const void *const *args) +{ +#define SAFE(c) (isalnum((c)) || \ + (c) == '/' || (c) == '-' || (c) == '_' || (c) == '.') + int i, len; + const char *str = *((const char **) (args[0])); + + for (i = len = 0; str[i]; ++i) { + if (!SAFE(str[i])) { + putc ('\\', stream); + len ++; + } + putc (str[i], stream); + len ++; + } + + return len; +} + +static int +print_sysroot_shell_quote (FILE *stream, + const struct printf_info *info, + const void *const *args) +{ +#define SAFE(c) (isalnum((c)) || \ + (c) == '/' || (c) == '-' || (c) == '_' || (c) == '.') + fputs (sysroot, stream); + return sysroot_len + print_shell_quote (stream, info, args); +} + +static int +print_arginfo (const struct printf_info *info, + size_t n, int *argtypes, int *size) +{ + if (n > 0) { + argtypes[0] = PA_STRING; + size[0] = sizeof (const char *); + } + return 1; +} + +#if 0 /* Quote 'in' for the shell, and write max len-1 bytes to out. The * result will be NUL-terminated, even if it is truncated. * @@ -721,6 +785,7 @@ shell_quote (char *out, int len, const char *in) return outlen; } +#endif /* Perform device name translation. Don't call this directly - * use the IS_DEVICE macro. diff --git a/daemon/initrd.c b/daemon/initrd.c index 392b811..59749bb 100644 --- a/daemon/initrd.c +++ b/daemon/initrd.c @@ -31,29 +31,23 @@ char ** do_initrd_list (char *path) { FILE *fp; - int len; char *cmd; char filename[PATH_MAX]; char **filenames = NULL; int size = 0, alloc = 0; + size_t len; NEED_ROOT (NULL); ABS_PATH (path, NULL); /* "zcat /sysroot/ | cpio --quiet -it", but path must be quoted. */ - len = 64 + sysroot_len + 2 * strlen (path); - cmd = malloc (len); - if (!cmd) { - reply_with_perror ("malloc"); + if (asprintf_nowarn (&cmd, "zcat %R | cpio --quiet -it", path) == -1) { + reply_with_perror ("asprintf"); return NULL; } - strcpy (cmd, "zcat "); - strcat (cmd, sysroot); - shell_quote (cmd+13, len-13, path); - strcat (cmd, " | cpio --quiet -it"); - - fprintf (stderr, "%s\n", cmd); + if (verbose) + fprintf (stderr, "%s\n", cmd); fp = popen (cmd, "r"); if (fp == NULL) { diff --git a/daemon/tar.c b/daemon/tar.c index 3008574..39b983c 100644 --- a/daemon/tar.c +++ b/daemon/tar.c @@ -38,7 +38,7 @@ fwrite_cb (void *fp_ptr, const void *buf, int len) int do_tar_in (char *dir) { - int err, r, len; + int err, r; FILE *fp; char *cmd; @@ -49,19 +49,16 @@ do_tar_in (char *dir) } /* "tar -C /sysroot%s -xf -" but we have to quote the dir. */ - len = 2 * strlen (dir) + sysroot_len + 32; - cmd = malloc (len); - if (!cmd) { + if (asprintf_nowarn (&cmd, "tar -C %R -xf -", dir) == -1) { err = errno; cancel_receive (); errno = err; - reply_with_perror ("malloc"); + reply_with_perror ("asprintf"); return -1; } - strcpy (cmd, "tar -C "); - strcat (cmd, sysroot); - shell_quote (cmd+15, len-15, dir); - strcat (cmd, " -xf -"); + + if (verbose) + fprintf (stderr, "%s\n", cmd); fp = popen (cmd, "w"); if (fp == NULL) { @@ -104,7 +101,7 @@ do_tar_in (char *dir) int do_tar_out (char *dir) { - int r, len; + int r; FILE *fp; char *cmd; char buf[GUESTFS_MAX_CHUNK_SIZE]; @@ -113,16 +110,13 @@ do_tar_out (char *dir) ABS_PATH (dir, -1); /* "tar -C /sysroot%s -cf - ." but we have to quote the dir. */ - len = 2 * strlen (dir) + sysroot_len + 32; - cmd = malloc (len); - if (!cmd) { - reply_with_perror ("malloc"); + if (asprintf_nowarn (&cmd, "tar -C %R -cf - .", dir) == -1) { + reply_with_perror ("asprintf"); return -1; } - strcpy (cmd, "tar -C "); - strcat (cmd, sysroot); - shell_quote (cmd+15, len-15, dir); - strcat (cmd, " -cf - ."); + + if (verbose) + fprintf (stderr, "%s\n", cmd); fp = popen (cmd, "r"); if (fp == NULL) { @@ -166,7 +160,7 @@ do_tar_out (char *dir) int do_tgz_in (char *dir) { - int err, r, len; + int err, r; FILE *fp; char *cmd; @@ -177,19 +171,16 @@ do_tgz_in (char *dir) } /* "tar -C /sysroot%s -zxf -" but we have to quote the dir. */ - len = 2 * strlen (dir) + sysroot_len + 32; - cmd = malloc (len); - if (!cmd) { + if (asprintf_nowarn (&cmd, "tar -C %R -zxf -", dir) == -1) { err = errno; cancel_receive (); errno = err; - reply_with_perror ("malloc"); + reply_with_perror ("asprintf"); return -1; } - strcpy (cmd, "tar -C "); - strcat (cmd, sysroot); - shell_quote (cmd+15, len-15, dir); - strcat (cmd, " -zxf -"); + + if (verbose) + fprintf (stderr, "%s\n", cmd); fp = popen (cmd, "w"); if (fp == NULL) { @@ -232,7 +223,7 @@ do_tgz_in (char *dir) int do_tgz_out (char *dir) { - int r, len; + int r; FILE *fp; char *cmd; char buf[GUESTFS_MAX_CHUNK_SIZE]; @@ -241,16 +232,13 @@ do_tgz_out (char *dir) ABS_PATH (dir, -1); /* "tar -C /sysroot%s -zcf - ." but we have to quote the dir. */ - len = 2 * strlen (dir) + sysroot_len + 32; - cmd = malloc (len); - if (!cmd) { - reply_with_perror ("malloc"); + if (asprintf_nowarn (&cmd, "tar -C %R -zcf - .", dir) == -1) { + reply_with_perror ("asprintf"); return -1; } - strcpy (cmd, "tar -C "); - strcat (cmd, sysroot); - shell_quote (cmd+15, len-15, dir); - strcat (cmd, " -zcf - ."); + + if (verbose) + fprintf (stderr, "%s\n", cmd); fp = popen (cmd, "r"); if (fp == NULL) { -- 1.6.2.5