[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] PATCH: Fix crash in QEMU driver with illegal machine/arch info



This bug was originally reported here

  https://bugzilla.redhat.com/show_bug.cgi?id=479203


If you define an guest VM specifying a architecture which does not 
actually exist in the QEMU driver's declared capabilities, then it
would dereference NULL when later attempting to start the VM.

The example from the bug, is attempting to create & start a 'x86_64'
VM on a host which is i686, and doesn't have the qemu-system-x86_64
emulator available. Or even just make up a random bad architecture.

This patch does two things

 - Makes the domain_conf.c parser validate the requested architecture
   against those actually available in the driver
 - Safety net in qemu_conf.c just in case the former validation somehow
   stopped working.

With this patch applie you now get

# virsh define demo.xml 
libvir: Domain Config error : internal error os type 'hvm' & arch 'x86_64' combination is not supported
error: Failed to define domain from demo.xml

Regards,
Daniel

diff -r e4ec5a7bf9df src/capabilities.c
--- a/src/capabilities.c	Thu Jan 29 11:54:55 2009 +0000
+++ b/src/capabilities.c	Thu Jan 29 13:56:55 2009 +0000
@@ -433,6 +433,30 @@ virCapabilitiesSupportsGuestOSType(virCa
 
 
 /**
+ * virCapabilitiesSupportsGuestOSType:
+ * @caps: capabilities to query
+ * @ostype: OS type to search for (eg 'hvm', 'xen')
+ * @arch: Architecture to search for (eg, 'i686', 'x86_64')
+ *
+ * Returns non-zero if the capabilities support the
+ * requested operating system type
+ */
+extern int
+virCapabilitiesSupportsGuestArch(virCapsPtr caps,
+                                 const char *ostype,
+                                 const char *arch)
+{
+    int i;
+    for (i = 0 ; i < caps->nguests ; i++) {
+        if (STREQ(caps->guests[i]->ostype, ostype) &&
+            STREQ(caps->guests[i]->arch.name, arch))
+            return 1;
+    }
+    return 0;
+}
+
+
+/**
  * virCapabilitiesDefaultGuestArch:
  * @caps: capabilities to query
  * @ostype: OS type to search for
diff -r e4ec5a7bf9df src/capabilities.h
--- a/src/capabilities.h	Thu Jan 29 11:54:55 2009 +0000
+++ b/src/capabilities.h	Thu Jan 29 13:56:55 2009 +0000
@@ -162,6 +162,12 @@ virCapabilitiesAddGuestFeature(virCapsGu
 extern int
 virCapabilitiesSupportsGuestOSType(virCapsPtr caps,
                                    const char *ostype);
+extern int
+virCapabilitiesSupportsGuestArch(virCapsPtr caps,
+                                 const char *ostype,
+                                 const char *arch);
+
+
 extern const char *
 virCapabilitiesDefaultGuestArch(virCapsPtr caps,
                                 const char *ostype);
diff -r e4ec5a7bf9df src/domain_conf.c
--- a/src/domain_conf.c	Thu Jan 29 11:54:55 2009 +0000
+++ b/src/domain_conf.c	Thu Jan 29 13:56:55 2009 +0000
@@ -2038,7 +2038,14 @@ static virDomainDefPtr virDomainDefParse
     }
 
     def->os.arch = virXPathString(conn, "string(./os/type[1]/@arch)", ctxt);
-    if (!def->os.arch) {
+    if (def->os.arch) {
+        if (!virCapabilitiesSupportsGuestArch(caps, def->os.type, def->os.arch)) {
+            virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                                 _("os type '%s' & arch '%s' combination is not supported"),
+                                 def->os.type, def->os.arch);
+            goto error;
+        }
+    } else {
         const char *defaultArch = virCapabilitiesDefaultGuestArch(caps, def->os.type);
         if (defaultArch == NULL) {
             virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
diff -r e4ec5a7bf9df src/qemu_conf.c
--- a/src/qemu_conf.c	Thu Jan 29 11:54:55 2009 +0000
+++ b/src/qemu_conf.c	Thu Jan 29 13:56:55 2009 +0000
@@ -843,8 +843,16 @@ int qemudBuildCommandLine(virConnectPtr 
 
     ADD_ARG_LIT(emulator);
     ADD_ARG_LIT("-S");
-    ADD_ARG_LIT("-M");
-    ADD_ARG_LIT(vm->def->os.machine);
+
+    /* This should *never* be NULL, since we always provide
+     * a machine in the capabilities data for QEMU. So this
+     * check is just here as a safety in case the unexpected
+     * happens */
+    if (vm->def->os.machine) {
+        ADD_ARG_LIT("-M");
+        ADD_ARG_LIT(vm->def->os.machine);
+    }
+
     if (disableKQEMU)
         ADD_ARG_LIT("-no-kqemu");
     ADD_ARG_LIT("-m");


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]