[libvirt] [PATCH] cpuCompare: Fix crash on unexpected CPU XML
When comparing a CPU without <model> element, such as <cpu> <topology sockets='1' cores='1' threads='1'/> </cpu> libvirt would happily crash without warning. --- src/cpu/cpu.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 279eee7..def6974 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -90,6 +90,12 @@ cpuCompareXML(virCPUDefPtr host, if (cpu == NULL) goto cleanup; + if (!cpu->model) { + virCPUReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("no CPU model specified")); + goto cleanup; + } + ret = cpuCompare(host, cpu); cleanup: -- 1.7.1.1
On Mon, Jul 12, 2010 at 05:50:24PM +0200, Jiri Denemark wrote:
When comparing a CPU without <model> element, such as
<cpu> <topology sockets='1' cores='1' threads='1'/> </cpu>
libvirt would happily crash without warning. --- src/cpu/cpu.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 279eee7..def6974 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -90,6 +90,12 @@ cpuCompareXML(virCPUDefPtr host, if (cpu == NULL) goto cleanup;
+ if (!cpu->model) { + virCPUReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("no CPU model specified")); + goto cleanup; + } + ret = cpuCompare(host, cpu);
cleanup:
Argh, ACK though that could have been checked one level down in cpuCompare() which could also make some checking about host and host->arch before dereferencing. Either it's an internal API where we trust the args or it's not but it's important to fix the crash either there or in cpuCompare Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
When comparing a CPU without <model> element, such as
<cpu> <topology sockets='1' cores='1' threads='1'/> </cpu>
libvirt would happily crash without warning. --- src/cpu/cpu.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 279eee7..def6974 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -90,6 +90,12 @@ cpuCompareXML(virCPUDefPtr host, if (cpu == NULL) goto cleanup;
+ if (!cpu->model) { + virCPUReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("no CPU model specified")); + goto cleanup; + } + ret = cpuCompare(host, cpu);
cleanup:
Argh, ACK
though that could have been checked one level down in cpuCompare() which could also make some checking about host and host->arch before dereferencing. Either it's an internal API where we trust the args or it's not but it's important to fix the crash either there or in cpuCompare
Well, both of them are internal. cpuCompare is used with virCPUDef structure created internally in libvirt so we trust the caller that it knows what it's doing. However, we should probably add some check to cpu* functions used internally to avoid troubles when we change something in the future. But that is a separate issue with lower priority, so I pushed this fix. Jirka
though that could have been checked one level down in cpuCompare() which could also make some checking about host and host->arch before dereferencing. Either it's an internal API where we trust the args or it's not but it's important to fix the crash either there or in cpuCompare
Well, both of them are internal.
Eh, I sent it without finishing this part... it must be the hot weather. I just wanted to add that cpuCompareXML is used for parsing user-supplied data making it more external while cpuCompare is for internal use only.
cpuCompare is used with virCPUDef structure created internally in libvirt so we trust the caller that it knows what it's doing. However, we should probably add some check to cpu* functions used internally to avoid troubles when we change something in the future.
But that is a separate issue with lower priority, so I pushed this fix.
Jirka
participants (2)
-
Daniel Veillard -
Jiri Denemark