The function init_dm_info() in xl_cmdimpl.c is used to initialise a
libxl_device_model_info structure with some default values. After being
called, some of those values are overridden. For the string values which
are not overwritten we either end up with a double free or attempt to
free a string literal resulting in a segfault. This type of usage model
would be complex to fix by adding strdup()'s in the init function and
then checking and freeing when over-writing.
My proposed solution is to add default versions of xlu_cfg_get_string
and xlu_cfg_get_long. This way both double-free's and leaks are avoided
and arguably the intention of the code is clearer.
Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
diff -r d323c34ec982 -r aa90461b1d04 tools/libxl/libxlu_cfg.c
--- a/tools/libxl/libxlu_cfg.c Tue Aug 31 13:30:21 2010 +0100
+++ b/tools/libxl/libxlu_cfg.c Tue Aug 31 14:01:07 2010 +0100
@@ -151,7 +151,23 @@ int xlu_cfg_get_string(const XLU_Config
*value_r= set->values[0];
return 0;
}
-
+
+int xlu_cfg_get_string_default(const XLU_Config *cfg, const char *n,
+ const char **value_r, const char *def) {
+ XLU_ConfigSetting *set;
+ int e;
+
+ e= find_atom(cfg,n,&set);
+ if (e) {
+ *value_r = strdup(def);
+ if ( def && NULL == *value_r )
+ return ENOMEM;
+ return 0;
+ }
+ *value_r= set->values[0];
+ return 0;
+}
+
int xlu_cfg_get_long(const XLU_Config *cfg, const char *n,
long *value_r) {
long l;
@@ -181,6 +197,35 @@ int xlu_cfg_get_long(const XLU_Config *c
return 0;
}
+int xlu_cfg_get_long_default(const XLU_Config *cfg, const char *n,
+ long *value_r, long def) {
+ long l;
+ XLU_ConfigSetting *set;
+ int e;
+ char *ep;
+
+ e= find_atom(cfg,n,&set); if (e) { *value_r = def; return 0; }
+ errno= 0; l= strtol(set->values[0], &ep, 0);
+ e= errno;
+ if (errno) {
+ e= errno;
+ assert(e==EINVAL || e==ERANGE);
+ fprintf(cfg->report,
+ "%s:%d: warning: parameter `%s' could not be parsed"
+ " as a number: %s\n",
+ cfg->filename, set->lineno, n, strerror(e));
+ *value_r = def;
+ return e;
+ }
+ if (*ep || ep==set->values[0]) {
+ fprintf(cfg->report,
+ "%s:%d: warning: parameter `%s' is not a valid number\n",
+ cfg->filename, set->lineno, n);
+ return EINVAL;
+ }
+ *value_r= l;
+ return 0;
+}
int xlu_cfg_get_list(const XLU_Config *cfg, const char *n,
XLU_ConfigList **list_r, int *entries_r) {
diff -r d323c34ec982 -r aa90461b1d04 tools/libxl/libxlutil.h
--- a/tools/libxl/libxlutil.h Tue Aug 31 13:30:21 2010 +0100
+++ b/tools/libxl/libxlutil.h Tue Aug 31 14:01:07 2010 +0100
@@ -46,7 +46,10 @@ void xlu_cfg_destroy(XLU_Config*);
*/
int xlu_cfg_get_string(const XLU_Config*, const char *n, const char **value_r);
+int xlu_cfg_get_string_default(const XLU_Config *cfg, const char *n,
+ const char **value_r, const char *def);
int xlu_cfg_get_long(const XLU_Config*, const char *n, long *value_r);
+int xlu_cfg_get_long_default(const XLU_Config*, const char *n, long *value_r,
long def);
int xlu_cfg_get_list(const XLU_Config*, const char *n,
XLU_ConfigList **list_r /* may be 0 */,
diff -r d323c34ec982 -r aa90461b1d04 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Tue Aug 31 13:30:21 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c Tue Aug 31 14:01:07 2010 +0100
@@ -286,36 +286,6 @@ static void init_build_info(libxl_domain
}
}
-static void init_dm_info(libxl_device_model_info *dm_info,
- libxl_domain_create_info *c_info, libxl_domain_build_info *b_info)
-{
- memset(dm_info, '\0', sizeof(*dm_info));
-
- libxl_uuid_generate(&dm_info->uuid);
-
- dm_info->dom_name = c_info->name;
- dm_info->device_model = "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;
- dm_info->vcpu_avail = b_info->cur_vcpus;
-
- dm_info->stdvga = 0;
- dm_info->vnc = 1;
- dm_info->vnclisten = "127.0.0.1";
- dm_info->vncdisplay = 0;
- dm_info->vncunused = 1;
- dm_info->keymap = NULL;
- dm_info->sdl = 0;
- dm_info->opengl = 0;
- dm_info->nographic = 0;
- dm_info->serial = NULL;
- dm_info->boot = "cda";
- dm_info->usb = 0;
- dm_info->usbdevice = NULL;
- dm_info->xen_platform_pci = 1;
-}
-
static void init_nic_info(libxl_device_nic *nic_info, int devnum)
{
const uint8_t *r;
@@ -1008,22 +978,31 @@ skip_vfb:
if (c_info->hvm == 1) {
/* init dm from c and b */
- init_dm_info(dm_info, c_info, b_info);
+ memset(dm_info, '\0', sizeof(*dm_info));
+
+ libxl_uuid_generate(&dm_info->uuid);
+ dm_info->dom_name = strdup(c_info->name);
+
+ dm_info->videoram = b_info->video_memkb / 1024;
+ dm_info->apic = b_info->u.hvm.apic;
+ dm_info->vcpus = b_info->max_vcpus;
+ dm_info->vcpu_avail = b_info->cur_vcpus;
/* then process config related to dm */
- if (!xlu_cfg_get_string (config, "device_model", &buf))
+ if (!xlu_cfg_get_string_default (config, "device_model", &buf,
"qemu-dm"))
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))
+ if (!xlu_cfg_get_long_default (config, "vnc", &l, 1))
dm_info->vnc = l;
- if (!xlu_cfg_get_string (config, "vnclisten", &buf))
+ if (!xlu_cfg_get_string_default (config, "vnclisten", &buf,
"127.0.0.1"))
dm_info->vnclisten = strdup(buf);
if (!xlu_cfg_get_string (config, "vncpasswd", &buf))
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))
+ if (!xlu_cfg_get_long_default (config, "vncunused", &l, 1))
dm_info->vncunused = l;
if (!xlu_cfg_get_string (config, "keymap", &buf))
dm_info->keymap = strdup(buf);
@@ -1035,7 +1014,7 @@ skip_vfb:
dm_info->nographic = l;
if (!xlu_cfg_get_string (config, "serial", &buf))
dm_info->serial = strdup(buf);
- if (!xlu_cfg_get_string (config, "boot", &buf))
+ if (!xlu_cfg_get_string_default (config, "boot", &buf, "cda"))
dm_info->boot = strdup(buf);
if (!xlu_cfg_get_long (config, "usb", &l))
dm_info->usb = l;
@@ -1043,7 +1022,7 @@ skip_vfb:
dm_info->usbdevice = strdup(buf);
if (!xlu_cfg_get_string (config, "soundhw", &buf))
dm_info->soundhw = strdup(buf);
- if (!xlu_cfg_get_long (config, "xen_platform_pci", &l))
+ if (!xlu_cfg_get_long_default (config, "xen_platform_pci", &l, 1))
dm_info->xen_platform_pci = l;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home