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@xxxxxxxxxx wrote: > >>From: Alex Jia<ajia@xxxxxxxxxx> > >> > >>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@xxxxxxxxxx> > >>--- > >> 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? > >> VIR_DOMAIN_CPUMASK_LEN)< 0) { > >>- memcpy(persistentDef->numatune.memory.nodemask, > >>- oldnodemask, VIR_DOMAIN_CPUMASK_LEN); > >>+ memcpy(params[i].value.s, oldnodemask, VIR_DOMAIN_CPUMASK_LEN); > >> ret = -1; > >> continue; > >> } > > From f2fe7c7d0041779895330416dca6ac97fcbec88a Mon Sep 17 00:00:00 2001 > >From: Hu Tao<hutao@xxxxxxxxxxxxxx> > >Date: Wed, 4 Jan 2012 17:00:27 +0800 > >Subject: [PATCH] qemu: fix a bug in numatune > > > >When setting numa nodeset for a domain which has no nodeset set > >before, libvirtd crashes by dereferring the pointer to the old > >nodemask which is null in the case. > >--- > > src/qemu/qemu_driver.c | 22 ++++++++++++++++++---- > > 1 files changed, 18 insertions(+), 4 deletions(-) > > > >diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > >index 82bab67..37330b4 100644 > >--- a/src/qemu/qemu_driver.c > >+++ b/src/qemu/qemu_driver.c > >@@ -6721,14 +6721,28 @@ qemuDomainSetNumaParameters(virDomainPtr dom, > > } > > > > if (flags& VIR_DOMAIN_AFFECT_CONFIG) { > >- memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, > >- VIR_DOMAIN_CPUMASK_LEN); > >+ bool savedmask = false; > >+ if (!persistentDef->numatune.memory.nodemask) { > >+ if (VIR_ALLOC_N(persistentDef->numatune.memory.nodemask, > >+ VIR_DOMAIN_CPUMASK_LEN)< 0) { > >+ virReportOOMError(); > >+ ret = -1; > >+ goto cleanup; > >+ } > >+ } else { > >+ memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, > >+ VIR_DOMAIN_CPUMASK_LEN); > >+ savedmask = true; > >+ } > > if (virDomainCpuSetParse(params[i].value.s, > > 0, > > persistentDef->numatune.memory.nodemask, > > VIR_DOMAIN_CPUMASK_LEN)< 0) { > >- memcpy(persistentDef->numatune.memory.nodemask, > >- oldnodemask, VIR_DOMAIN_CPUMASK_LEN); > >+ if (savedmask) > >+ memcpy(persistentDef->numatune.memory.nodemask, > >+ oldnodemask, VIR_DOMAIN_CPUMASK_LEN); > >+ else > >+ VIR_FREE(persistentDef->numatune.memory.nodemask); Oops. The same should be done when --live. > > ret = -1; > > continue; > > } -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list