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

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