From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gianni Tedesco <gianni.tedesco@citrix.com>
Subject: [PATCH]: xl: don't free string literals
Date: Mon, 6 Sep 2010 13:17:53 +0100
Message-ID: <1283775473.20276.135.camel@qabil.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Xen Devel <xen-devel@lists.xensource.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
List-Id: xen-devel@lists.xenproject.org

The function init_dm_info() is initialising some strings from literals.
This is bad juju because when the destructor is called we cannot know if
the string literal was overridden with a strdup()'d value. Therefore
strdup the initialisers in init_dm_info() and unconditionally free them
before assigning non-default values to prevent their leakage.

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 5f53805b349e -r e954e6b6d311 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Fri Sep 03 18:44:49 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Mon Sep 06 13:13:46 2010 +0100
@@ -294,7 +294,7 @@ static void init_dm_info(libxl_device_mo
     libxl_uuid_generate(&dm_info->uuid);
 
     dm_info->dom_name = c_info->name;
-    dm_info->device_model = "qemu-dm";
+    dm_info->device_model = strdup("qemu-dm");
     dm_info->videoram = b_info->video_memkb / 1024;
     dm_info->apic = b_info->u.hvm.apic;
     dm_info->vcpus = b_info->max_vcpus;
@@ -302,7 +302,7 @@ static void init_dm_info(libxl_device_mo
 
     dm_info->stdvga = 0;
     dm_info->vnc = 1;
-    dm_info->vnclisten = "127.0.0.1";
+    dm_info->vnclisten = strdup("127.0.0.1");
     dm_info->vncdisplay = 0;
     dm_info->vncunused = 1;
     dm_info->keymap = NULL;
@@ -310,7 +310,7 @@ static void init_dm_info(libxl_device_mo
     dm_info->opengl = 0;
     dm_info->nographic = 0;
     dm_info->serial = NULL;
-    dm_info->boot = "cda";
+    dm_info->boot = strdup("cda");
     dm_info->usb = 0;
     dm_info->usbdevice = NULL;
     dm_info->xen_platform_pci = 1;
@@ -1019,38 +1019,54 @@ skip_vfb:
         init_dm_info(dm_info, c_info, b_info);
 
         /* then process config related to dm */
-        if (!xlu_cfg_get_string (config, "device_model", &buf))
+        if (!xlu_cfg_get_string (config, "device_model", &buf)) {
+            free(dm_info->device_model);
             dm_info->device_model = strdup(buf);
+        }
         if (!xlu_cfg_get_long (config, "stdvga", &l))
             dm_info->stdvga = l;
         if (!xlu_cfg_get_long (config, "vnc", &l))
             dm_info->vnc = l;
-        if (!xlu_cfg_get_string (config, "vnclisten", &buf))
+        if (!xlu_cfg_get_string (config, "vnclisten", &buf)) {
+            free(dm_info->vnclisten);
             dm_info->vnclisten = strdup(buf);
-        if (!xlu_cfg_get_string (config, "vncpasswd", &buf))
+        }
+        if (!xlu_cfg_get_string (config, "vncpasswd", &buf)) {
+            free(dm_info->vncpasswd);
             dm_info->vncpasswd = strdup(buf);
+        }
         if (!xlu_cfg_get_long (config, "vncdisplay", &l))
             dm_info->vncdisplay = l;
         if (!xlu_cfg_get_long (config, "vncunused", &l))
             dm_info->vncunused = l;
-        if (!xlu_cfg_get_string (config, "keymap", &buf))
+        if (!xlu_cfg_get_string (config, "keymap", &buf)) {
+            free(dm_info->keymap);
             dm_info->keymap = strdup(buf);
+        }
         if (!xlu_cfg_get_long (config, "sdl", &l))
             dm_info->sdl = l;
         if (!xlu_cfg_get_long (config, "opengl", &l))
             dm_info->opengl = l;
         if (!xlu_cfg_get_long (config, "nographic", &l))
             dm_info->nographic = l;
-        if (!xlu_cfg_get_string (config, "serial", &buf))
+        if (!xlu_cfg_get_string (config, "serial", &buf)) {
+            free(dm_info->serial);
             dm_info->serial = strdup(buf);
-        if (!xlu_cfg_get_string (config, "boot", &buf))
+        }
+        if (!xlu_cfg_get_string (config, "boot", &buf)) {
+            free(dm_info->boot);
             dm_info->boot = strdup(buf);
+        }
         if (!xlu_cfg_get_long (config, "usb", &l))
             dm_info->usb = l;
-        if (!xlu_cfg_get_string (config, "usbdevice", &buf))
+        if (!xlu_cfg_get_string (config, "usbdevice", &buf)) {
+            free(dm_info->usbdevice);
             dm_info->usbdevice = strdup(buf);
-        if (!xlu_cfg_get_string (config, "soundhw", &buf))
+        }
+        if (!xlu_cfg_get_string (config, "soundhw", &buf)) {
+            free(dm_info->soundhw);
             dm_info->soundhw = strdup(buf);
+        }
         if (!xlu_cfg_get_long (config, "xen_platform_pci", &l))
             dm_info->xen_platform_pci = l;
     }