1:53 a.m.
From: Alex Jia <ajia(a)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
% 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(a)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,
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;
}
--
1.7.1
3:04 a.m.
New subject: [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 03:53:19PM +0800, ajia(a)redhat.com wrote:
From: Alex Jia <ajia(a)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(a)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.
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(a)cn.fujitsu.com>
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);
ret = -1;
continue;
}
--
1.7.4.4
--
Thanks,
Hu Tao
3:15 a.m.
New subject: [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:04 PM, Hu Tao wrote:
On Wed, Jan 04, 2012 at 03:53:19PM +0800, ajia(a)redhat.com wrote:
> From: Alex Jia<ajia(a)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(a)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
>
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(a)cn.fujitsu.com>
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);
ret = -1;
continue;
}
3:28 a.m.
New subject: [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: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<ajia(a)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(a)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?
>>
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(a)cn.fujitsu.com>
>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
3:56 a.m.
New subject: [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<ajia(a)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(a)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
numa_mode : strict
numa_nodeset : 0-1
numa_mode : strict
numa_nodeset : 0-2
numa_mode : strict
numa_nodeset : 0-3
numa_mode : strict
numa_nodeset : 0-4
numa_mode : strict
numa_nodeset : 0-5
numa_mode : strict
numa_nodeset : 0-6
numa_mode : strict
numa_nodeset : 0-7
numa_mode : strict
numa_nodeset : 0-8
numa_mode : strict
numa_nodeset : 0-9
numa_mode : strict
numa_nodeset : 0-10
>>>
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(a)cn.fujitsu.com>
>> 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;
>> }
7:12 p.m.
New subject: [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<ajia(a)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(a)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
9:03 p.m.
New subject: [libvirt] [PATCH] qemu: fix a bug in numatune (was Re: [PATCH] qemu: Prevent crash of libvirtd when setting numa parameters)
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(a)redhat.com wrote:
>>>>> From: Alex Jia<ajia(a)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(a)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
3:41 a.m.
New subject: [libvirt] [PATCHv2] 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 | 45 +++++++++++++++++++++++++++++++++++++--------
1 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82bab67..036d252 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6679,6 +6679,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
}
} else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) {
int rc;
+ bool savedmask;
char oldnodemask[VIR_DOMAIN_CPUMASK_LEN];
if (param->type != VIR_TYPED_PARAM_STRING) {
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -6707,28 +6708,56 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
/* update vm->def here so that dumpxml can read the new
* values from vm->def. */
- memcpy(oldnodemask, vm->def->numatune.memory.nodemask,
- VIR_DOMAIN_CPUMASK_LEN);
+ savedmask = false;
+ if (!vm->def->numatune.memory.nodemask) {
+ if (VIR_ALLOC_N(vm->def->numatune.memory.nodemask,
+ VIR_DOMAIN_CPUMASK_LEN) < 0) {
+ virReportOOMError();
+ ret = -1;
+ goto cleanup;
+ }
+ } else {
+ memcpy(oldnodemask, vm->def->numatune.memory.nodemask,
+ VIR_DOMAIN_CPUMASK_LEN);
+ savedmask = true;
+ }
if (virDomainCpuSetParse(params[i].value.s,
0,
vm->def->numatune.memory.nodemask,
VIR_DOMAIN_CPUMASK_LEN) < 0) {
- memcpy(vm->def->numatune.memory.nodemask,
- oldnodemask, VIR_DOMAIN_CPUMASK_LEN);
+ if (savedmask)
+ memcpy(vm->def->numatune.memory.nodemask,
+ oldnodemask, VIR_DOMAIN_CPUMASK_LEN);
+ else
+ VIR_FREE(vm->def->numatune.memory.nodemask);
ret = -1;
continue;
}
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
- memcpy(oldnodemask, persistentDef->numatune.memory.nodemask,
- VIR_DOMAIN_CPUMASK_LEN);
+ 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);
ret = -1;
continue;
}
--
1.7.4.4
2:06 p.m.
New subject: [libvirt] [PATCHv2] qemu: fix a bug in numatune
On 01/04/2012 02:41 AM, Hu Tao wrote:
When setting numa nodeset for a domain which has no nodeset set
before, libvirtd crashes by dereferring the pointer to the old
s/dereferring/dereferencing/
nodemask which is null in the case.
---
src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++--------
1 files changed, 37 insertions(+), 8 deletions(-)
ACK and pushed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4705
days inactive
4706
days old
8 comments
4 participants
participants (4)
-
ajiaï¼ redhat.com -
Alex Jia
-
Eric Blake -
Hu Tao