From 01ae3678f1f9ca5007a8e1c23a1b12b046b342e9 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Thu, 28 Sep 2006 19:20:52 +0000 Subject: [PATCH] Fixed buffer overflow in populating CPU<->VCPU mapping. Cleanup whitespace --- ChangeLog | 10 ++++++ src/xend_internal.c | 83 +++++++++++++++++++++++---------------------- 2 files changed, 53 insertions(+), 40 deletions(-) diff --git a/ChangeLog b/ChangeLog index 021d5308c4..05c4ca899c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +Thu Sep 21 10:19:02 EDT 2006 Daniel Berrange + + * src/xend_internal.c: Check if the physical CPU will fit in the maplen + provided by the caller when populating VCPU<->CPU mapping. This is because + XenD will return data for 32 physical CPUs, even if the box only has 4 + CPUs. The caller of course will only have allocated a map big enough for + the actual number of physical CPUs. We simply check against maplen param + supplied by caller & discard info about CPUs which don't fit. Also santise + whitespace. + Fri Sep 22 11:02:48 CEST 2006 Daniel Veillard * docs/* libvirt.spec.in configure.in NEWS: preparing release of 0.1.6 diff --git a/src/xend_internal.c b/src/xend_internal.c index 015900a927..672f7e0c22 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -2653,7 +2653,7 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, */ int xenDaemonDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, - unsigned char *cpumaps, int maplen) + unsigned char *cpumaps, int maplen) { struct sexpr *root, *s, *t; virVcpuInfoPtr ipt = info; @@ -2662,14 +2662,14 @@ xenDaemonDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, int vcpu, cpu; if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) - || (info == NULL) || (maxinfo < 1)) { + || (info == NULL) || (maxinfo < 1)) { virXendError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG, - __FUNCTION__); + __FUNCTION__); return (-1); } if (cpumaps != NULL && maplen < 1) { virXendError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG, - __FUNCTION__); + __FUNCTION__); return (-1); } root = sexpr_get(domain->conn, "/xend/domain/%s?op=vcpuinfo", domain->name); @@ -2677,46 +2677,49 @@ xenDaemonDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, return (-1); if (cpumaps != NULL) - memset(cpumaps, 0, maxinfo * maplen); + memset(cpumaps, 0, maxinfo * maplen); /* scan the sexprs from "(vcpu (number x)...)" and get parameter values */ - for (s = root; s->kind == SEXPR_CONS; s = s->cdr) - if ((s->car->kind == SEXPR_CONS) && - (s->car->car->kind == SEXPR_VALUE) && - !strcmp(s->car->car->value, "vcpu")) { - t = s->car; - vcpu = ipt->number = sexpr_int(t, "vcpu/number"); - if ((oln = sexpr_int(t, "vcpu/online")) != 0) { - if (sexpr_int(t, "vcpu/running")) ipt->state = VIR_VCPU_RUNNING; - if (sexpr_int(t, "vcpu/blocked")) ipt->state = VIR_VCPU_BLOCKED; - } - else ipt->state = VIR_VCPU_OFFLINE; - ipt->cpuTime = sexpr_float(t, "vcpu/cpu_time") * 1000000000; - ipt->cpu = oln ? sexpr_int(t, "vcpu/cpu") : -1; - - if (cpumaps != NULL && vcpu >= 0 && vcpu < maxinfo) { - cpumap = (unsigned char *) VIR_GET_CPUMAP(cpumaps, maplen, vcpu); - /* - * get sexpr from "(cpumap (x y z...))" and convert values - * to bitmap - */ - for (t = t->cdr; t->kind == SEXPR_CONS; t = t->cdr) - if ((t->car->kind == SEXPR_CONS) && - (t->car->car->kind == SEXPR_VALUE) && - !strcmp(t->car->car->value, "cpumap") && - (t->car->cdr->kind == SEXPR_CONS)) { - for (t = t->car->cdr->car; t->kind == SEXPR_CONS; t = t->cdr) - if (t->car->kind == SEXPR_VALUE) { - cpu = strtol(t->car->value, NULL, 0); - if (cpu >= 0) - VIR_USE_CPU(cpumap, cpu); - } - break; + for (s = root; s->kind == SEXPR_CONS; s = s->cdr) { + if ((s->car->kind == SEXPR_CONS) && + (s->car->car->kind == SEXPR_VALUE) && + !strcmp(s->car->car->value, "vcpu")) { + t = s->car; + vcpu = ipt->number = sexpr_int(t, "vcpu/number"); + if ((oln = sexpr_int(t, "vcpu/online")) != 0) { + if (sexpr_int(t, "vcpu/running")) ipt->state = VIR_VCPU_RUNNING; + if (sexpr_int(t, "vcpu/blocked")) ipt->state = VIR_VCPU_BLOCKED; } - } + else + ipt->state = VIR_VCPU_OFFLINE; + ipt->cpuTime = sexpr_float(t, "vcpu/cpu_time") * 1000000000; + ipt->cpu = oln ? sexpr_int(t, "vcpu/cpu") : -1; - if (++nbinfo == maxinfo) break; - ipt++; + if (cpumaps != NULL && vcpu >= 0 && vcpu < maxinfo) { + cpumap = (unsigned char *) VIR_GET_CPUMAP(cpumaps, maplen, vcpu); + /* + * get sexpr from "(cpumap (x y z...))" and convert values + * to bitmap + */ + for (t = t->cdr; t->kind == SEXPR_CONS; t = t->cdr) + if ((t->car->kind == SEXPR_CONS) && + (t->car->car->kind == SEXPR_VALUE) && + !strcmp(t->car->car->value, "cpumap") && + (t->car->cdr->kind == SEXPR_CONS)) { + for (t = t->car->cdr->car; t->kind == SEXPR_CONS; t = t->cdr) + if (t->car->kind == SEXPR_VALUE) { + cpu = strtol(t->car->value, NULL, 0); + if (cpu >= 0 && (VIR_CPU_MAPLEN(cpu+1) <= maplen)) { + VIR_USE_CPU(cpumap, cpu); + } + } + break; + } + } + + if (++nbinfo == maxinfo) break; + ipt++; + } } sexpr_free(root); return(nbinfo);