launch: don't add a drive twice
[libguestfs.git] / src / launch.c
index ce26e25..8a64b5e 100644 (file)
@@ -32,6 +32,8 @@
 #include <time.h>
 #include <sys/stat.h>
 #include <sys/select.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 #include <dirent.h>
 #include <signal.h>
 #include <assert.h>
@@ -233,17 +235,21 @@ guestfs__config (guestfs_h *g,
  * O_DIRECT.  This fails on some filesystem types (notably tmpfs).
  * So we check if we can open the file with or without O_DIRECT,
  * and use cache=off (or not) accordingly.
+ *
+ * NB: This function is only called on the !readonly path.  We must
+ * try to open with O_RDWR to test that the file is readable and
+ * writable here.
  */
 static int
 test_cache_off (guestfs_h *g, const char *filename)
 {
-  int fd = open (filename, O_RDONLY|O_DIRECT);
+  int fd = open (filename, O_RDWR|O_DIRECT);
   if (fd >= 0) {
     close (fd);
     return 1;
   }
 
-  fd = open (filename, O_RDONLY);
+  fd = open (filename, O_RDWR);
   if (fd >= 0) {
     close (fd);
     return 0;
@@ -279,6 +285,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   char *format;
   char *iface;
   char *name;
+  char *abs_path = NULL;
   int use_cache_off;
 
   if (strchr (filename, ',') != NULL) {
@@ -298,18 +305,12 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   if (format && !valid_format_iface (format)) {
     error (g, _("%s parameter is empty or contains disallowed characters"),
            "format");
-    free (format);
-    free (iface);
-    free (name);
-    return -1;
+    goto err_out;
   }
   if (!valid_format_iface (iface)) {
     error (g, _("%s parameter is empty or contains disallowed characters"),
            "iface");
-    free (format);
-    free (iface);
-    free (name);
-    return -1;
+    goto err_out;
   }
 
   /* For writable files, see if we can use cache=off.  This also
@@ -317,36 +318,44 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
    * to do the check explicitly.
    */
   use_cache_off = readonly ? 0 : test_cache_off (g, filename);
-  if (use_cache_off == -1) {
-    free (format);
-    free (iface);
-    free (name);
-    return -1;
-  }
+  if (use_cache_off == -1)
+    goto err_out;
 
   if (readonly) {
-    if (access (filename, F_OK) == -1) {
+    if (access (filename, R_OK) == -1) {
       perrorf (g, "%s", filename);
-      free (format);
-      free (iface);
-      free (name);
-      return -1;
+      goto err_out;
     }
   }
 
+  abs_path = realpath (filename, NULL);
   struct drive **i = &(g->drives);
-  while (*i != NULL) i = &((*i)->next);
+  while (*i != NULL) {
+    if (STREQ((*i)->path, abs_path)) {
+      error (g, _("drive %s can't be added twice"), abs_path);
+      goto err_out;
+    }
+    i = &((*i)->next);
+  }
 
   *i = safe_malloc (g, sizeof (struct drive));
   (*i)->next = NULL;
-  (*i)->path = safe_strdup (g, filename);
+  (*i)->path = safe_strdup (g, abs_path);
   (*i)->readonly = readonly;
   (*i)->format = format;
   (*i)->iface = iface;
   (*i)->name = name;
   (*i)->use_cache_off = use_cache_off;
 
+  free (abs_path);
   return 0;
+
+err_out:
+  free (format);
+  free (iface);
+  free (name);
+  free (abs_path);
+  return -1;
 }
 
 int
@@ -560,6 +569,16 @@ launch_appliance (guestfs_h *g)
     alloc_cmdline (g);
     g->cmdline[0] = g->qemu;
 
+    /* CVE-2011-4127 mitigation: Disable SCSI ioctls on virtio-blk
+     * devices.  The -global option must exist, but you can pass any
+     * strings to it so we don't need to check for the specific virtio
+     * feature.
+     */
+    if (qemu_supports (g, "-global")) {
+      add_cmdline (g, "-global");
+      add_cmdline (g, "virtio-blk-pci.scsi=off");
+    }
+
     /* Add drives */
     struct drive *drv = g->drives;
     while (drv != NULL) {
@@ -582,7 +601,19 @@ launch_appliance (guestfs_h *g)
      */
     if (qemu_supports (g, "-machine")) {
       add_cmdline (g, "-machine");
+#if QEMU_MACHINE_TYPE_IS_BROKEN
+      /* Workaround for qemu 0.15: We have to add the '[type=]pc'
+       * since there is no default.  This is not a permanent solution
+       * because this only works on PC-like hardware.  Other platforms
+       * like ppc would need a different machine type.
+       *
+       * This bug is fixed in qemu commit 2645c6dcaf6ea2a51a, and was
+       * not a problem in qemu < 0.15.
+       */
+      add_cmdline (g, "pc,accel=kvm:tcg");
+#else
       add_cmdline (g, "accel=kvm:tcg");
+#endif
     } else {
       /* qemu sometimes needs this option to enable hardware
        * virtualization, but some versions of 'qemu-kvm' will use KVM
@@ -853,6 +884,13 @@ launch_appliance (guestfs_h *g)
   if (r == -1)
     goto cleanup1;
 
+  /* NB: We reach here just because qemu has opened the socket.  It
+   * does not mean the daemon is up until we read the
+   * GUESTFS_LAUNCH_FLAG below.  Failures in qemu startup can still
+   * happen even if we reach here, even early failures like not being
+   * able to open a drive.
+   */
+
   close (g->sock); /* Close the listening socket. */
   g->sock = r; /* This is the accepted data socket. */
 
@@ -1059,6 +1097,34 @@ guestfs___persistent_tmpdir (void)
   return tmpdir;
 }
 
+/* Recursively remove a temporary directory.  If removal fails, just
+ * return (it's a temporary directory so it'll eventually be cleaned
+ * up by a temp cleaner).  This is done using "rm -rf" because that's
+ * simpler and safer, but we have to exec to ensure that paths don't
+ * need to be quoted.
+ */
+void
+guestfs___remove_tmpdir (const char *dir)
+{
+  pid_t pid = fork ();
+
+  if (pid == -1) {
+    perror ("remove tmpdir: fork");
+    return;
+  }
+  if (pid == 0) {
+    execlp ("rm", "rm", "-rf", dir, NULL);
+    perror ("remove tmpdir: exec: rm");
+    _exit (EXIT_FAILURE);
+  }
+
+  /* Parent. */
+  if (waitpid (pid, NULL, 0) == -1) {
+    perror ("remove tmpdir: waitpid");
+    return;
+  }
+}
+
 /* Compute Y - X and return the result in milliseconds.
  * Approximately the same as this code:
  * http://www.mpp.mpg.de/~huber/util/timevaldiff.c