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

[Hu Tao]

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)

-- 
Thanks,
Hu Tao