Currently, an array of libxl_string_list (char **) or in other words, a triple char pointer is initialized. This is dereferenced to a char ** type and stored in serial_list, which is NULL at this point. There is an attempt to reference an element of this serial_list when making a call to libxlMakeChrdevStr which causes a segmentation fault. To fix this, we simply allocate an array of char * instead of libxl_string_list. This patch also adds testcases to extend coverage over both single serial and multiple serial cases. Signed-off-by: Rayhan Faizel <rayhan.faizel(a)gmail.com&gt; --- src/libxl/libxl_conf.c | 2 +- .../multiple-serial.json | 63 +++++++++++++++++++ .../multiple-serial.xml | 47 ++++++++++++++ .../libxlxml2domconfigdata/single-serial.json | 52 +++++++++++++++ .../libxlxml2domconfigdata/single-serial.xml | 25 ++++++++ tests/libxlxml2domconfigtest.c | 3 + 6 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 tests/libxlxml2domconfigdata/multiple-serial.json create mode 100644 tests/libxlxml2domconfigdata/multiple-serial.xml create mode 100644 tests/libxlxml2domconfigdata/single-serial.json create mode 100644 tests/libxlxml2domconfigdata/single-serial.xml diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 62e1be6672..8c91489ffd 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -692,7 +692,7 @@ libxlMakeDomBuildInfo(virDomainDef *def, 0) return -1; } else { - b_info->u.hvm.serial_list = *g_new0(libxl_string_list, def->nserials + 1); + b_info->u.hvm.serial_list = g_new0(char *, def->nserials + 1); for (i = 0; i < def->nserials; i++) { if (libxlMakeChrdevStr(def->serials[i], &b_info->u.hvm.serial_list[i]) < 0) diff --git a/tests/libxlxml2domconfigdata/multiple-serial.json b/tests/libxlxml2domconfigdata/multiple-serial.json new file mode 100644 index 0000000000..121f2d1260 --- /dev/null +++ b/tests/libxlxml2domconfigdata/multiple-serial.json @@ -0,0 +1,63 @@ +{ + "c_info": { + "type": "hvm", + "name": "test-hvm", + "uuid": "2147d599-9cc6-c0dc-92ab-4064b5446e9b" + }, + "b_info": { + "max_vcpus": 4, + "avail_vcpus": [ + 0, + 1, + 2, + 3 + ], + "max_memkb": 1048576, + "target_memkb": 1048576, + "shadow_memkb": 1234, + "sched_params": { + + }, + "acpi": "True", + "apic": "True", + "type.hvm": { + "pae": "True", + "nographic": "True", + "vga": { + "kind": "none" + }, + "vnc": { + "enable": "False" + }, + "sdl": { + "enable": "False" + }, + "spice": { + + }, + "serial_list": [ + "null", + "stdio", + "vc", + "pty", + "pipe:/tmp/file", + "file:/tmp/serial.log", + "/dev/ttyS2", + "udp::9999@:0", + "tcp:127.0.0.1:9999", + "unix:/tmp/serial-server.sock,server,nowait" + ], + "boot": "c", + "rdm": { + + } + }, + "arch_arm": { + + }, + "arch_x86": { + + } + }, + "on_reboot": "restart" +} diff --git a/tests/libxlxml2domconfigdata/multiple-serial.xml b/tests/libxlxml2domconfigdata/multiple-serial.xml new file mode 100644 index 0000000000..c50ffd0bb4 --- /dev/null +++ b/tests/libxlxml2domconfigdata/multiple-serial.xml @@ -0,0 +1,47 @@ +<domain type='xen'> + <name>test-hvm</name> + <description>None</description> + <uuid>2147d599-9cc6-c0dc-92ab-4064b5446e9b</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>4</vcpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <clock offset='utc'/> + <os> + <type>hvm</type> + <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='hd'/> + </os> + <features> + <apic/> + <acpi/> + <pae/> + </features> + <devices> + <serial type='null'/> + <serial type='stdio'/> + <serial type='vc'/> + <serial type='pty'/> + <serial type='pipe'> + <source path='/tmp/file'/> + </serial> + <serial type='file'> + <source path='/tmp/serial.log'/> + </serial> + <serial type='dev'> + <source path='/dev/ttyS2'/> + </serial> + <serial type='udp'> + <source mode='connect' service='9999'/> + </serial> + <serial type='tcp'> + <source mode='connect' host='127.0.0.1' service='9999'/> + <protocol type='raw'/> + </serial> + <serial type='unix'> + <source mode='bind' path='/tmp/serial-server.sock'/> + </serial> + </devices> +</domain> diff --git a/tests/libxlxml2domconfigdata/single-serial.json b/tests/libxlxml2domconfigdata/single-serial.json new file mode 100644 index 0000000000..a736e6f805 --- /dev/null +++ b/tests/libxlxml2domconfigdata/single-serial.json @@ -0,0 +1,52 @@ +{ + "c_info": { + "type": "hvm", + "name": "test-hvm", + "uuid": "2147d599-9cc6-c0dc-92ab-4064b5446e9b" + }, + "b_info": { + "max_vcpus": 4, + "avail_vcpus": [ + 0, + 1, + 2, + 3 + ], + "max_memkb": 1048576, + "target_memkb": 1048576, + "shadow_memkb": 1234, + "sched_params": { + + }, + "acpi": "True", + "apic": "True", + "type.hvm": { + "pae": "True", + "nographic": "True", + "vga": { + "kind": "none" + }, + "vnc": { + "enable": "False" + }, + "sdl": { + "enable": "False" + }, + "spice": { + + }, + "serial": "pty", + "boot": "c", + "rdm": { + + } + }, + "arch_arm": { + + }, + "arch_x86": { + + } + }, + "on_reboot": "restart" +} diff --git a/tests/libxlxml2domconfigdata/single-serial.xml b/tests/libxlxml2domconfigdata/single-serial.xml new file mode 100644 index 0000000000..f468024189 --- /dev/null +++ b/tests/libxlxml2domconfigdata/single-serial.xml @@ -0,0 +1,25 @@ +<domain type='xen'> + <name>test-hvm</name> + <description>None</description> + <uuid>2147d599-9cc6-c0dc-92ab-4064b5446e9b</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>4</vcpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <clock offset='utc'/> + <os> + <type>hvm</type> + <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='hd'/> + </os> + <features> + <apic/> + <acpi/> + <pae/> + </features> + <devices> + <serial type='pty'/> + </devices> +</domain> diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index 21c4e7d149..255855b156 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -208,6 +208,9 @@ mymain(void) DO_TEST("max-eventchannels-hvm"); + DO_TEST("single-serial"); + DO_TEST("multiple-serial"); + unlink("libxl-driver.log"); testXLFreeDriver(driver); -- 2.34.1

libxlMakeVfb always succeeds regardless of if the graphics type is actually supported or not. libxl_defbool_val is called in libxlMakeBuildInfoVfb which besides returning the boolean value of the defbool also has an assertion that the defbool value is not set to default. It is possible to fail this assertion if an unsupported graphics type is used. In libxlMakeVfb, the VNC and SDL enable defbools are still left in their default state if the graphics type falls outside the two, which leads to this issue. This patch adds a check to reject graphics types outside of SDL, VNC, and SPICE very early on in libxlMakeVfb. As a safeguard, we also initialize both vnc enable and sdl enable defbools as false early. Signed-off-by: Rayhan Faizel <rayhan.faizel(a)gmail.com&gt; --- src/libxl/libxl_conf.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 8c91489ffd..c404226e43 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1506,18 +1506,18 @@ libxlMakeVfb(virPortAllocatorRange *graphicsports, virDomainGraphicsListenDef *glisten = NULL; libxl_device_vfb_init(x_vfb); + libxl_defbool_set(&x_vfb->sdl.enable, 0); + libxl_defbool_set(&x_vfb->vnc.enable, 0); switch (l_vfb->type) { case VIR_DOMAIN_GRAPHICS_TYPE_SDL: libxl_defbool_set(&x_vfb->sdl.enable, 1); - libxl_defbool_set(&x_vfb->vnc.enable, 0); libxl_defbool_set(&x_vfb->sdl.opengl, 0); x_vfb->sdl.display = g_strdup(l_vfb->data.sdl.display); x_vfb->sdl.xauthority = g_strdup(l_vfb->data.sdl.xauth); break; case VIR_DOMAIN_GRAPHICS_TYPE_VNC: libxl_defbool_set(&x_vfb->vnc.enable, 1); - libxl_defbool_set(&x_vfb->sdl.enable, 0); /* driver handles selection of free port */ libxl_defbool_set(&x_vfb->vnc.findunused, 0); if (l_vfb->data.vnc.autoport) { @@ -1542,13 +1542,18 @@ libxlMakeVfb(virPortAllocatorRange *graphicsports, x_vfb->keymap = g_strdup(l_vfb->data.vnc.keymap); break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + break; + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: case VIR_DOMAIN_GRAPHICS_TYPE_DBUS: case VIR_DOMAIN_GRAPHICS_TYPE_LAST: - break; + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported graphics type %1$s"), + virDomainGraphicsTypeToString(l_vfb->type)); + return -1; } return 0; -- 2.34.1