From 0a742856490bfdcb02c2af48a2a849593cccf1c7 Mon Sep 17 00:00:00 2001

From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

Date: Thu, 29 Aug 2013 17:18:49 +0200

Subject: [PATCH 08/48] libxkutil: Improve domain.os_info cleanup

The union fields in os_info were set by means of XML parsing which

doesn't take into account that certain fields are depending on the

virtualization type.

This could lead both to memory overwrites and memory leaks.

Fixed by using temporary variables and type-based setting of fields

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

Signed-off-by: John Ferlan <jferlan@redhat.com>

---

 libxkutil/device_parsing.c | 73 +++++++++++++++++++++++++++++++++-------------

 1 file changed, 52 insertions(+), 21 deletions(-)

diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c

index 542e4e9..ad0f19c 100644

--- a/libxkutil/device_parsing.c

+++ b/libxkutil/device_parsing.c

@@ -1103,23 +1103,37 @@ int parse_fq_devid(const char *devid, char **host, char **device)

         return 1;

 }

+static void cleanup_bootlist(char **blist, unsigned blist_ct)

+{

+        while (blist_ct > 0) {

+                free(blist[--blist_ct]);

+        }

+        free(blist);

+}

+

 static int parse_os(struct domain *dominfo, xmlNode *os)

 {

         xmlNode *child;

         char **blist = NULL;

         unsigned bl_size = 0;

+        char *kernel = NULL;

+        char *initrd = NULL;

+        char *cmdline = NULL;

+        char *loader = NULL;

+        char *boot = NULL;

+        char *init = NULL;

         for (child = os->children; child != NULL; child = child->next) {

-                if (XSTREQ(child->name, "type"))

+                if (XSTREQ(child->name, "type")) {

                         STRPROP(dominfo, os_info.pv.type, child);

-                else if (XSTREQ(child->name, "kernel"))

-                        STRPROP(dominfo, os_info.pv.kernel, child);

+                } else if (XSTREQ(child->name, "kernel"))

+                        kernel = get_node_content(child);

                 else if (XSTREQ(child->name, "initrd"))

-                        STRPROP(dominfo, os_info.pv.initrd, child);

+                        initrd = get_node_content(child);

                 else if (XSTREQ(child->name, "cmdline"))

-                        STRPROP(dominfo, os_info.pv.cmdline, child);

+                        cmdline = get_node_content(child);

                 else if (XSTREQ(child->name, "loader"))

-                        STRPROP(dominfo, os_info.fv.loader, child);

+                        loader = get_node_content(child);

                 else if (XSTREQ(child->name, "boot")) {

                         char **tmp_list = NULL;

@@ -1137,7 +1151,7 @@ static int parse_os(struct domain *dominfo, xmlNode *os)

                         blist[bl_size] = get_attr_value(child, "dev");

                         bl_size++;

                 } else if (XSTREQ(child->name, "init"))

-                        STRPROP(dominfo, os_info.lxc.init, child);

+                        init = get_node_content(child);

         }

         if ((STREQC(dominfo->os_info.fv.type, "hvm")) &&

@@ -1154,17 +1168,39 @@ static int parse_os(struct domain *dominfo, xmlNode *os)

         else

                 dominfo->type = -1;

-        if (STREQC(dominfo->os_info.fv.type, "hvm")) {

+        switch (dominfo->type) {

+        case DOMAIN_XENFV:

+        case DOMAIN_KVM:

+        case DOMAIN_QEMU:

+                dominfo->os_info.fv.loader = loader;

                 dominfo->os_info.fv.bootlist_ct = bl_size;

                 dominfo->os_info.fv.bootlist = blist;

-        } else {

-            int i;

-

-            for (i = 0; i < bl_size; i++)

-                free(blist[i]);

-            free(blist);

+                loader = NULL;

+                blist = NULL;

+                bl_size = 0;

+                break;

+        case DOMAIN_XENPV:

+                dominfo->os_info.pv.kernel = kernel;

+                dominfo->os_info.pv.initrd = initrd;

+                dominfo->os_info.pv.cmdline = cmdline;

+                kernel = NULL;

+                initrd = NULL;

+                cmdline = NULL;

+                break;

+        case DOMAIN_LXC:

+                dominfo->os_info.lxc.init = init;

+                init = NULL;

+                break;

+        default:

+                break;

         }

+        free(kernel);

+        free(initrd);

+        free(cmdline);

+        free(boot);

+        free(init);

+        cleanup_bootlist(blist, bl_size);

         return 1;

 }

@@ -1360,15 +1396,10 @@ void cleanup_dominfo(struct domain **dominfo)

                 free(dom->os_info.pv.cmdline);

         } else if ((dom->type == DOMAIN_XENFV) ||

                    (dom->type == DOMAIN_KVM) || (dom->type == DOMAIN_QEMU)) {

-                int i;

-

                 free(dom->os_info.fv.type);

                 free(dom->os_info.fv.loader);

-                

-                for (i = 0; i < dom->os_info.fv.bootlist_ct; i++) {

-                        free(dom->os_info.fv.bootlist[i]);

-                } 

-                free(dom->os_info.fv.bootlist);

+                cleanup_bootlist(dom->os_info.fv.bootlist,

+                                 dom->os_info.fv.bootlist_ct);

         } else if (dom->type == DOMAIN_LXC) {

                 free(dom->os_info.lxc.type);

                 free(dom->os_info.lxc.init);

-- 

1.8.5.3