1. Patchew
  2. Libvirt
  3. [PATCH 0/2] libxl_conf: Fix crashes in libxl config generation
  4. View patch

[PATCH 1/2] libxl_conf: Fix config generation for multiple serial devices

Rayhan Faizel posted 2 patches 1 year, 4 months ago
[PATCH 1/2] libxl_conf: Fix config generation for multiple serial devices
Posted by Rayhan Faizel 1 year, 4 months ago 
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@gmail.com>
---
 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
Re: [PATCH 1/2] libxl_conf: Fix config generation for multiple serial devices
Posted by Martin Kletzander 1 year, 4 months ago 
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@gmail.com>
>---
> 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@redhat.com>

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
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.