On Tue, Sep 17, 2024 at 11:28:45PM +0530, Rayhan Faizel wrote:
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@xxxxxxxxx>
---
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)
right below this line, in case of an error, is a call to: libxl_string_list_dispose(&b_info->u.hvm.serial_list); but since we are the ones constructing it I think it would be safer to also dispose of it with our functions as I could not find what and how that libxl_ function does that. Anyway, this is strictly better than before, so the adjustment can be done in a separate patch. So both of these are Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx> and I'll push them in a while. Thanks for the patches.
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
Attachment:
signature.asc
Description: PGP signature
