On 12/05/10 16:01, Richard W.M. Jones wrote:
>From fb8ae842ef1195fe3896e3f83a97762d27364abb Mon Sep 17 00:00:00
2001
From: Richard Jones <rjones(a)redhat.com>
Date: Wed, 12 May 2010 15:58:00 +0100
Subject: [PATCH] guestfish -i and virt-inspector work on filenames containing spaces
(RHBZ#507810).
This commit fixes a long-standing bug which prevented guestfish -i
and virt-inspector from working on disk images which had a space
in the filename (or other unsafe characters). It works by ensuring
that the strings passed between guestfish -i and virt-inspector are
quoted correctly in both directions.
Note that this commit adds a dependency from virt-inspector to
the perl module String::ShellQuote. We have previously used this
module in virt-make-fs.
---
fish/fish.c | 111 ++++++++++++++++++++++++++++++++++++----------
inspector/virt-inspector | 9 +++-
2 files changed, 94 insertions(+), 26 deletions(-)
diff --git a/fish/fish.c b/fish/fish.c
index 75bc2cf..34188d4 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -1,5 +1,5 @@
/* guestfish - the filesystem interactive shell
- * Copyright (C) 2009 Red Hat Inc.
+ * Copyright (C) 2009-2010 Red Hat Inc.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -66,6 +66,7 @@ static void cmdline (char *argv[], int optind, int argc);
static void initialize_readline (void);
static void cleanup_readline (void);
static void add_history_line (const char *);
+static int print_shell_quote (FILE *stream, const char *str);
/* Currently open libguestfs handle. */
guestfs_h *g;
@@ -362,9 +363,6 @@ main (int argc, char *argv[])
/* Inspector mode invalidates most of the other arguments. */
if (inspector) {
- char cmd[1024];
- int r;
-
if (drvs || mps || remote_control_listen || remote_control ||
guestfs_get_selinux (g)) {
fprintf (stderr, _("%s: cannot use -i option with -a, -m, -N, "
@@ -379,44 +377,90 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- strcpy (cmd, "a=`virt-inspector");
+ char *cmd;
+ size_t cmdlen;
+ FILE *fp = open_memstream (&cmd, &cmdlen);
+ if (fp == NULL) {
+ perror ("open_memstream");
+ exit (EXIT_FAILURE);
+ }
+
+ //fprintf (fp, "virt-inspector");
+ fprintf (fp, "inspector/run-inspector-locally");
I don't think ^^^ is still supposed to be there.
while (optind < argc) {
- if (strlen (cmd) + strlen (argv[optind]) + strlen (real_argv0) + 60
- >= sizeof cmd) {
- fprintf (stderr,
- _("%s: virt-inspector command too long for fixed-size
buffer\n"),
- program_name);
- exit (EXIT_FAILURE);
- }
- strcat (cmd, " '");
- strcat (cmd, argv[optind]);
- strcat (cmd, "'");
+ fputc (' ', fp);
+ print_shell_quote (fp, argv[optind]);
optind++;
}
if (read_only)
- strcat (cmd, " --ro-fish");
+ fprintf (fp, " --ro-fish");
else
- strcat (cmd, " --fish");
+ fprintf (fp, " --fish");
+
+ if (fclose (fp) == -1) {
+ perror ("fclose");
+ exit (EXIT_FAILURE);
+ }
+
+ if (verbose)
+ fprintf (stderr,
+ "%s -i: running: %s\n", program_name, cmd);
+
+ FILE *pp = popen (cmd, "r");
+ if (pp == NULL) {
+ perror (cmd);
+ exit (EXIT_FAILURE);
+ }
+
+ char *cmd2;
+ fp = open_memstream (&cmd2, &cmdlen);
+ if (fp == NULL) {
+ perror ("open_memstream");
+ exit (EXIT_FAILURE);
+ }
- sprintf (&cmd[strlen(cmd)], "` && %s $a", real_argv0);
+ fprintf (fp, "%s", real_argv0);
if (guestfs_get_verbose (g))
- strcat (cmd, " -v");
+ fprintf (fp, " -v");
if (!guestfs_get_autosync (g))
- strcat (cmd, " -n");
+ fprintf (fp, " -n");
if (guestfs_get_trace (g))
- strcat (cmd, " -x");
+ fprintf (fp, " -x");
+
+ char *insp = NULL;
+ size_t insplen;
+ if (getline (&insp, &insplen, pp) == -1) {
+ perror (cmd);
+ exit (EXIT_FAILURE);
+ }
+ fprintf (fp, " %s", insp);
+
+ if (pclose (pp) == -1) {
+ perror (cmd);
+ exit (EXIT_FAILURE);
+ }
+
+ if (fclose (fp) == -1) {
+ perror ("fclose");
+ exit (EXIT_FAILURE);
+ }
if (verbose)
fprintf (stderr,
- "%s -i: running virt-inspector command:\n%s\n", program_name,
cmd);
+ "%s -i: running: %s\n", program_name, cmd2);
- r = system (cmd);
+ int r = system (cmd2);
Incidentally, why don't we just exec() here?
if (r == -1) {
- perror ("system");
+ perror (cmd2);
exit (EXIT_FAILURE);
}
+
+ free (cmd);
+ free (cmd2);
+ free (insp);
+
exit (WEXITSTATUS (r));
}
@@ -1616,3 +1660,22 @@ file_out (const char *arg)
}
return ret;
}
+
+static int
+print_shell_quote (FILE *stream, const char *str)
+{
+#define SAFE(c) (c_isalnum((c)) || \
+ (c) == '/' || (c) == '-' || (c) == '_' || (c)
== '.')
+ int i, len;
+
+ for (i = len = 0; str[i]; ++i) {
+ if (!SAFE(str[i])) {
+ putc ('\\', stream);
+ len ++;
+ }
+ putc (str[i], stream);
+ len ++;
+ }
+
+ return len;
+}
The return value of print_shell_quote isn't used. No need to track len.
diff --git a/inspector/virt-inspector b/inspector/virt-inspector
index 4428ecd..f62d21f 100755
--- a/inspector/virt-inspector
+++ b/inspector/virt-inspector
@@ -27,6 +27,7 @@ use Pod::Usage;
use Getopt::Long;
use Data::Dumper;
use XML::Writer;
+use String::ShellQuote qw(shell_quote);
String::ShellQuote isn't in RHEL 5. Are you really sure you want this?
use Locale::TextDomain 'libguestfs';
# Optional:
@@ -298,13 +299,17 @@ if ($output eq "fish" || $output eq "ro-fish")
{
print "--ro ";
}
- print "-a $_ " foreach @images;
+ foreach (@images) {
+ printf "-a %s ", shell_quote ($_);
+ }
my $mounts = $oses->{$root_dev}->{mounts};
# Have to mount / first. Luckily '/' is early in the ASCII
# character set, so this should be OK.
foreach (sort keys %$mounts) {
- print "-m $mounts->{$_}:$_ " if $_ ne "swap" &&
$_ ne "none";
+ if ($_ ne "swap" && $_ ne "none") {
+ printf "-m %s ", shell_quote ("$mounts->{$_}:$_");
+ }
}
print "\n"
}
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490