[libvirt] [PATCH] qemu: fix libvirtd crash when querying halted cpus info

[Peter Krempa]

On Wed, Nov 02, 2016 at 18:56:49 +0300, Maxim Nestratov wrote:
> It was introduced by commit 7a51d9ebb, which started to use
> monitor commands without job acquiring, which is unsafe and leads
> to simultaneous access to vm->mon structure by different threads.
> 
> Crash backtrace is the following (shortened):
> 
> Program received signal SIGSEGV, Segmentation fault.
> qemuMonitorSend (mon=mon at entry=0x7f4ef4000d20, msg=msg at entry=0x7f4f18e78640) at qemu/qemu_monitor.c:1011
> 1011        while (!mon->msg->finished) {
> 
> 0  qemuMonitorSend () at qemu/qemu_monitor.c:1011
> 1  0x00007f691abdc720 in qemuMonitorJSONCommandWithFd () at qemu/qemu_monitor_json.c:298
> 2  0x00007f691abde64a in qemuMonitorJSONCommand at qemu/qemu_monitor_json.c:328
> 3  qemuMonitorJSONQueryCPUs at qemu/qemu_monitor_json.c:1408
> 4  0x00007f691abcaebd in qemuMonitorGetCPUInfo g at entry=false) at qemu/qemu_monitor.c:1931
> 5  0x00007f691ab96863 in qemuDomainRefreshVcpuHalted at qemu/qemu_domain.c:6309
> 6  0x00007f691ac0af99 in qemuDomainGetStatsVcpu at qemu/qemu_driver.c:18945
> 7  0x00007f691abef921 in qemuDomainGetStats  at qemu/qemu_driver.c:19469
> 8  qemuConnectGetAllDomainStats at qemu/qemu_driver.c:19559
> 9  0x00007f693382e806 in virConnectGetAllDomainStats at libvirt-domain.c:11546
> 10 0x00007f6934470c40 in remoteDispatchConnectGetAllDomainStats at remote.c:6267
> 
> (gdb) p mon->msg
> $1 = (qemuMonitorMessagePtr) 0x0
> 
> This change fixes it by calling qemuDomainRefreshVcpuHalted only when job is acquired.
> 
> Signed-off-by: Maxim Nestratov <mnestratov at virtuozzo.com>
> ---
>  src/qemu/qemu_driver.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index dd1907b..43a546d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18915,7 +18915,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
>                         virDomainObjPtr dom,
>                         virDomainStatsRecordPtr record,
>                         int *maxparams,
> -                       unsigned int privflags ATTRIBUTE_UNUSED)
> +                       unsigned int privflags)
>  {
>      size_t i;
>      int ret = -1;
> @@ -18939,14 +18939,20 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
>          return -1;
>  
>      if (VIR_ALLOC_N(cpuinfo, virDomainDefGetVcpus(dom->def)) < 0 ||
> -        VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
> -        goto cleanup;
> -
> -    if (qemuDomainRefreshVcpuHalted(driver, dom,
> -                                    QEMU_ASYNC_JOB_NONE) == 0 &&
> +        VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0 ||
>          VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0)
>          goto cleanup;
>  
> +    if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) {
> +

Spurious empty line.

> +        if (qemuDomainRefreshVcpuHalted(driver, dom,
> +                                        QEMU_ASYNC_JOB_NONE) < 0) {
> +            /* it's ok to be silent and go ahead, because halted vcpu info
> +             * wasn't here from the beginning */
> +            virResetLastError();
> +        }
> +    }
> +

Not visible in this context: The output of the parameter needs to be
suppressed if the parameter can't be requested.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161115/0c6edf17/attachment-0001.sig>