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

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(a)redhat.com wrote: >>>>> From: Alex Jia&lt;ajia(a)redhat.com&gt; >>>>> >>>>> 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&lt;ajia(a)redhat.com&gt; >>>>> --- >>>>> 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)