The refactoring in commit de49d5bad3 accidentally dropped the statement
setting def to NULL after successfully adding it to the virDomainObjList,
causing it to be freed while still in use. The resulting memory
corruption caused unpredictable behavior, often resulting in a libvirtd
crash.

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

Unpredictable is an understatement! When running monolithic libvirtd with
both qemu and xen drviers enabled, qemu crashed while initializing. Recall
it is initialized after xen.

Thread 17 "daemon-init" received signal SIGSEGV, Segmentation fault.
#0  0x00007f32e5fbe9e3 in _int_malloc () at /lib64/libc.so.6
#1  0x00007f32e5fbf6e0 in _int_realloc () at /lib64/libc.so.6
#2  0x00007f32e5fc0729 in realloc () at /lib64/libc.so.6
#3  0x00007f32e6dc21b8 in g_realloc () at /usr/lib64/libglib-2.0.so.0
#4  0x00007f32e7532090 in virReallocN (ptrptr=0x7f329affcad8, size=1, count=1403)
    at ../src/util/viralloc.c:91
#5  0x00007f32e75530c7 in virCommandProcessIO (cmd=0x7f328807ff40) at ../src/util/vircommand.c:2271
#6  0x00007f32e7553a6a in virCommandRun (cmd=0x7f328807ff40, exitstatus=0x0)
    at ../src/util/vircommand.c:2451
#7  0x00007f32e75dde73 in virSysinfoReadDMI () at ../src/util/virsysinfo.c:1237
#8  0x00007f32e75de0cb in virSysinfoRead () at ../src/util/virsysinfo.c:1294
#9  0x00007f32a240b69d in qemuStateInitialize
    (privileged=true, root=0x0, callback=0x56453a0b3e97 <daemonInhibitCallback>, opaque=0x56453b000030) at ../src/qemu/qemu_driver.c:658
#10 0x00007f32e7832350 in virStateInitialize
    (privileged=true, mandatory=false, root=0x0, callback=0x56453a0b3e97 <daemonInhibitCallback>, opaque=0x56453b000030) at ../src/libvirt.c:656
#11 0x000056453a0b4175 in daemonRunStateInit (opaque=0x56453b000030)
    at ../src/remote/remote_daemon.c:596

 src/libxl/libxl_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 083738871d..571b70f982 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -627,6 +627,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
                                        NULL)))
             goto cleanup;
 
+        def = NULL;
         vm->persistent = 1;
         virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
     }
-- 
2.28.0

On Thu, Sep 24, 2020 at 10:57:59 -0600, Jim Fehlig wrote:
> The refactoring in commit de49d5bad3 accidentally dropped the statement
> setting def to NULL after successfully adding it to the virDomainObjList,
> causing it to be freed while still in use. The resulting memory
> corruption caused unpredictable behavior, often resulting in a libvirtd
> crash.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>

For 6.8.0:
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

On Thu, Sep 24, 2020 at 1:03 PM Jim Fehlig <jfehlig@suse.com> wrote:
>
> The refactoring in commit de49d5bad3 accidentally dropped the statement
> setting def to NULL after successfully adding it to the virDomainObjList,
> causing it to be freed while still in use. The resulting memory
> corruption caused unpredictable behavior, often resulting in a libvirtd
> crash.
>
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>
> Unpredictable is an understatement! When running monolithic libvirtd with
> both qemu and xen drviers enabled, qemu crashed while initializing. Recall
> it is initialized after xen.
>
> Thread 17 "daemon-init" received signal SIGSEGV, Segmentation fault.
> #0  0x00007f32e5fbe9e3 in _int_malloc () at /lib64/libc.so.6
> #1  0x00007f32e5fbf6e0 in _int_realloc () at /lib64/libc.so.6
> #2  0x00007f32e5fc0729 in realloc () at /lib64/libc.so.6
> #3  0x00007f32e6dc21b8 in g_realloc () at /usr/lib64/libglib-2.0.so.0
> #4  0x00007f32e7532090 in virReallocN (ptrptr=0x7f329affcad8, size=1, count=1403)
>     at ../src/util/viralloc.c:91
> #5  0x00007f32e75530c7 in virCommandProcessIO (cmd=0x7f328807ff40) at ../src/util/vircommand.c:2271
> #6  0x00007f32e7553a6a in virCommandRun (cmd=0x7f328807ff40, exitstatus=0x0)
>     at ../src/util/vircommand.c:2451
> #7  0x00007f32e75dde73 in virSysinfoReadDMI () at ../src/util/virsysinfo.c:1237
> #8  0x00007f32e75de0cb in virSysinfoRead () at ../src/util/virsysinfo.c:1294
> #9  0x00007f32a240b69d in qemuStateInitialize
>     (privileged=true, root=0x0, callback=0x56453a0b3e97 <daemonInhibitCallback>, opaque=0x56453b000030) at ../src/qemu/qemu_driver.c:658
> #10 0x00007f32e7832350 in virStateInitialize
>     (privileged=true, mandatory=false, root=0x0, callback=0x56453a0b3e97 <daemonInhibitCallback>, opaque=0x56453b000030) at ../src/libvirt.c:656
> #11 0x000056453a0b4175 in daemonRunStateInit (opaque=0x56453b000030)
>     at ../src/remote/remote_daemon.c:596
>
>  src/libxl/libxl_driver.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 083738871d..571b70f982 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -627,6 +627,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
>                                         NULL)))
>              goto cleanup;
>
> +        def = NULL;
>          vm->persistent = 1;
>          virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
>      }
> --
> 2.28.0
>
>

Reviewed-by: Neal Gompa <ngompa13@gmail.com>


-- 
真実はいつも一つ！/ Always, there's only one truth!