[libvirt] [PATCH] qemu: Prevent crash of libvirtd without guest agent configuration

[Alex Jia]

On 07/16/2013 04:35 PM, Peter Krempa wrote:
> On 07/16/13 08:31, Alex Jia wrote:
>> If users haven't configured guest agent then qemuAgentCommand() will
>> dereference a NULL 'mon' pointer, which causes crash of libvirtd.
>
> uhh, right :/ I broke that :/
>
>>
>> GDB backtrace:
>>
>> (gdb) bt
>> #0  virNetServerFatalSignal (sig=11, siginfo=<value optimized out>, 
>> context=<value optimized out>) at rpc/virnetserver.c:326
>> #1 <signal handler called>
>> #2  qemuAgentCommand (mon=0x0, cmd=0x7f39300017b0, 
>> reply=0x7f394b090910, seconds=-2) at qemu/qemu_agent.c:975
>> #3  0x00007f39429507f6 in qemuAgentGetVCPUs (mon=0x0, 
>> info=0x7f394b0909b8) at qemu/qemu_agent.c:1475
>> #4  0x00007f39429d9857 in qemuDomainGetVcpusFlags (dom=<value 
>> optimized out>, flags=9) at qemu/qemu_driver.c:4849
>> #5  0x00007f3957dffd8d in virDomainGetVcpusFlags 
>> (domain=0x7f39300009c0, flags=8) at libvirt.c:9843
>>
>> How to reproduce?
>>
>> # To start a guest without guest agent configuration
>> # then run the following cmdline
>>
>> # virsh vcpucount foobar --guest
>> error: End of file while reading data: Input/output error
>> error: One or more references were leaked after disconnect from the 
>> hypervisor
>> error: Failed to reconnect to the hypervisor
>>
>> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=984821
>>
>> Signed-off-by: Alex Jia <ajia at redhat.com>
>> ---
>>   src/qemu/qemu_driver.c |   12 ++++++++++++
>>   1 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 495867a..1448aa7 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -4122,6 +4122,12 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, 
>> unsigned int nvcpus,
>>               goto endjob;
>>           }
>>
>> +        if (!priv->agent) {
>> +            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>> +                           _("QEMU guest agent is not configured"));
>> +            goto cleanup;
>> +        }
>> +
>
> We probably also want:
>
>         if (priv->agentError) {
>             virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
>                            _("QEMU guest agent is not "
>                              "available due to an error"));
>             goto cleanup;
>         }

Yes, I will add it into v2, thanks for your review.

>         if (!priv->agent) {
>             virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>                            _("QEMU guest agent is not configured"));
>             goto cleanup;
>         }
>
>
>
>
>>           qemuDomainObjEnterAgent(vm);
>>           ncpuinfo = qemuAgentGetVCPUs(priv->agent, &cpuinfo);
>>           qemuDomainObjExitAgent(vm);
>
> Peter
>