[libvirt] [PATCH] qemu: fix a bug in numatune (was Re: [PATCH] qemu: Prevent crash of libvirtd when setting numa parameters)

[Alex Jia]

On 01/05/2012 09:12 AM, Hu Tao wrote:
> On Wed, Jan 04, 2012 at 05:56:21PM +0800, Alex Jia wrote:
>> On 01/04/2012 05:28 PM, Hu Tao wrote:
>>> On Wed, Jan 04, 2012 at 05:15:24PM +0800, Alex Jia wrote:
>>>> On 01/04/2012 05:04 PM, Hu Tao wrote:
>>>>> On Wed, Jan 04, 2012 at 03:53:19PM +0800, ajia at redhat.com wrote:
>>>>>> From: Alex Jia<ajia at redhat.com>
>>>>>>
>>>>>> It's a NULL pointer deref issue, which leads to libvirtd crash. This patch
>>>>>> directly use 'params[i].value.s' value instead of derefing a NULL pointer
>>>>>> on memcpy.
>>>>>>
>>>>>> * how to reproduce?
>>>>>> % virsh numatune<domain>    --nodeset 0
>>>>> The domain must have no nodeset set previously (to crash in this example).
>>>>>
>>>>>> % service libvirtd status
>>>>>>
>>>>>> * src/qemu/qemu_driver.c (qemuDomainSetNumaParameters): avoid a NULL pointer deref.
>>>>>>
>>>>>> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771562
>>>>>>
>>>>>> Signed-off-by: Alex Jia<ajia at redhat.com>
>>>>>> ---
>>>>>>   src/qemu/qemu_driver.c |    6 ++----
>>>>>>   1 files changed, 2 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>>>>> index 82bab67..1bd93f6 100644
>>>>>> --- a/src/qemu/qemu_driver.c
>>>>>> +++ b/src/qemu/qemu_driver.c
>>>>>> @@ -6721,14 +6721,12 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
>>>>>>               }
>>>>>>
>>>>>>               if (flags&    VIR_DOMAIN_AFFECT_CONFIG) {
>>>>>> -                memcpy(oldnodemask, persistentDef->numatune.memory.nodemask,
>>>>>> -                       VIR_DOMAIN_CPUMASK_LEN);
>>>>>> +                memcpy(oldnodemask, params[i].value.s, VIR_DOMAIN_CPUMASK_LEN);
>>>>>>                   if (virDomainCpuSetParse(params[i].value.s,
>>>>>>                                            0,
>>>>>>                                            persistentDef->numatune.memory.nodemask,
>>>>> Not correct. In this case persistentDef->numatune.memory.nodemask is
>>>>> null, and virDomainCpuSetParse will always fail, thus the nodeset will
>>>>> never be set.
>>>> In fact, I can successfully set nodeset value:
>>>>
>>>> # virsh numatune foo --nodeset 0-1
>>>>
>>>> # virsh numatune foo
>>>> numa_mode      : strict
>>>> numa_nodeset   : 0-1
>>> Weird. I've never succeeded with your patch. Can you double-check again?
>> Hu Tao, Indeed, it's weird. the patch always works well for me:
>>
>> # for i in $(seq 10); do virsh numatune foo --nodeset 0-$i; virsh
>> numatune foo; done
> Can you test as the following steps?
>
> 1. remove numatune element from the dom's xml.
> 2. restart libvirtd
> 3. set numa nodeset (say, virsh numatune dom --nodeset 0-2)
Yeah, you're right, I need to cleanup previous <numatune> element block 
from guest, and your patch '[PATCHv2] qemu: fix a bug in numatune' works 
well for me.

Thanks for your comment,
Alex