xenconfig: fix handling of NULL disk source

[libvirt] [PATCH v2] xenconfig: fix handling of NULL disk source

Posted by Wim Ten Have 6 years, 11 months ago

From: Wim ten Have <wim.ten.have@oracle.com>

It is possible to crash libvirtd when converting xl native config to
domXML when the xl config contains an empty disk source, e.g. an empty
CDROM. Fix by checking that the disk source is non-NULL before parsing it.

Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
---
 src/xenconfig/xen_xl.c                                    | 4 ++++
 tests/xlconfigdata/test-disk-positional-parms-partial.cfg | 2 +-
 tests/xlconfigdata/test-disk-positional-parms-partial.xml | 6 ++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 4f24d45..cac440c 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -316,6 +316,10 @@ xenParseXLDiskSrc(virDomainDiskDefPtr disk, char *srcstr)
     char *tmpstr = NULL;
     int ret = -1;
 
+    /* A NULL source is valid, e.g. an empty CDROM */
+    if (srcstr == NULL)
+        return 0;
+
     if (STRPREFIX(srcstr, "rbd:")) {
         if (!(tmpstr = virStringReplace(srcstr, "\\\\", "\\")))
             goto cleanup;
diff --git a/tests/xlconfigdata/test-disk-positional-parms-partial.cfg b/tests/xlconfigdata/test-disk-positional-parms-partial.cfg
index fd16db0..940304e 100644
--- a/tests/xlconfigdata/test-disk-positional-parms-partial.cfg
+++ b/tests/xlconfigdata/test-disk-positional-parms-partial.cfg
@@ -22,4 +22,4 @@ parallel = "none"
 serial = "none"
 builder = "hvm"
 boot = "d"
-disk = [ "/dev/HostVG/XenGuest2,,hda,,backendtype=phy", "/var/lib/libvirt/images/XenGuest2-home,,hdb,,", "/root/boot.iso,,hdc,,devtype=cdrom" ]
+disk = [ "/dev/HostVG/XenGuest2,,hda,,backendtype=phy", "/var/lib/libvirt/images/XenGuest2-home,,hdb,,", "/root/boot.iso,,hdc,,devtype=cdrom" , "format=raw,vdev=hdd,access=ro,devtype=cdrom" ]
diff --git a/tests/xlconfigdata/test-disk-positional-parms-partial.xml b/tests/xlconfigdata/test-disk-positional-parms-partial.xml
index e86a5be..52b21dc 100644
--- a/tests/xlconfigdata/test-disk-positional-parms-partial.xml
+++ b/tests/xlconfigdata/test-disk-positional-parms-partial.xml
@@ -39,6 +39,12 @@
       <readonly/>
       <address type='drive' controller='0' bus='1' target='0' unit='0'/>
     </disk>
+    <disk type='file' device='cdrom'>
+      <driver name='qemu' type='raw'/>
+      <target dev='hdd' bus='ide'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='1' target='0' unit='1'/>
+    </disk>
     <controller type='ide' index='0'/>
     <interface type='bridge'>
       <mac address='00:16:3e:66:92:9c'/>
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] xenconfig: fix handling of NULL disk source

Posted by Jim Fehlig 6 years, 11 months ago

On 05/19/2017 08:38 AM, Wim Ten Have wrote:
> From: Wim ten Have <wim.ten.have@oracle.com>
>
> It is possible to crash libvirtd when converting xl native config to
> domXML when the xl config contains an empty disk source, e.g. an empty
> CDROM. Fix by checking that the disk source is non-NULL before parsing it.
>
> Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
> ---
>  src/xenconfig/xen_xl.c                                    | 4 ++++
>  tests/xlconfigdata/test-disk-positional-parms-partial.cfg | 2 +-
>  tests/xlconfigdata/test-disk-positional-parms-partial.xml | 6 ++++++
>  3 files changed, 11 insertions(+), 1 deletion(-)

Reviewed-by: Jim Fehlig <jfehlig@suse.com>

and pushed. Thanks!

Regards,
Jim

>
> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> index 4f24d45..cac440c 100644
> --- a/src/xenconfig/xen_xl.c
> +++ b/src/xenconfig/xen_xl.c
> @@ -316,6 +316,10 @@ xenParseXLDiskSrc(virDomainDiskDefPtr disk, char *srcstr)
>      char *tmpstr = NULL;
>      int ret = -1;
>
> +    /* A NULL source is valid, e.g. an empty CDROM */
> +    if (srcstr == NULL)
> +        return 0;
> +
>      if (STRPREFIX(srcstr, "rbd:")) {
>          if (!(tmpstr = virStringReplace(srcstr, "\\\\", "\\")))
>              goto cleanup;
> diff --git a/tests/xlconfigdata/test-disk-positional-parms-partial.cfg b/tests/xlconfigdata/test-disk-positional-parms-partial.cfg
> index fd16db0..940304e 100644
> --- a/tests/xlconfigdata/test-disk-positional-parms-partial.cfg
> +++ b/tests/xlconfigdata/test-disk-positional-parms-partial.cfg
> @@ -22,4 +22,4 @@ parallel = "none"
>  serial = "none"
>  builder = "hvm"
>  boot = "d"
> -disk = [ "/dev/HostVG/XenGuest2,,hda,,backendtype=phy", "/var/lib/libvirt/images/XenGuest2-home,,hdb,,", "/root/boot.iso,,hdc,,devtype=cdrom" ]
> +disk = [ "/dev/HostVG/XenGuest2,,hda,,backendtype=phy", "/var/lib/libvirt/images/XenGuest2-home,,hdb,,", "/root/boot.iso,,hdc,,devtype=cdrom" , "format=raw,vdev=hdd,access=ro,devtype=cdrom" ]
> diff --git a/tests/xlconfigdata/test-disk-positional-parms-partial.xml b/tests/xlconfigdata/test-disk-positional-parms-partial.xml
> index e86a5be..52b21dc 100644
> --- a/tests/xlconfigdata/test-disk-positional-parms-partial.xml
> +++ b/tests/xlconfigdata/test-disk-positional-parms-partial.xml
> @@ -39,6 +39,12 @@
>        <readonly/>
>        <address type='drive' controller='0' bus='1' target='0' unit='0'/>
>      </disk>
> +    <disk type='file' device='cdrom'>
> +      <driver name='qemu' type='raw'/>
> +      <target dev='hdd' bus='ide'/>
> +      <readonly/>
> +      <address type='drive' controller='0' bus='1' target='0' unit='1'/>
> +    </disk>
>      <controller type='ide' index='0'/>
>      <interface type='bridge'>
>        <mac address='00:16:3e:66:92:9c'/>
>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list