On Thu, Aug 30, 2012 at 03:51:54PM +0200, Peter Krempa wrote:
virDomainVcpuPinDefCopy when the control flow reaches out of memory cleanup code, the flow would end in a infinite loop as the loop variable wasn't decremented.
Also a dereference of NULL pointers was possible if allocation of the Vcpu pinning definiton structure failed. --- src/conf/domain_conf.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 224aec5..2dad64d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1496,7 +1496,7 @@ virDomainVcpuPinDefPtr * virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin) { int i = 0; - virDomainVcpuPinDefPtr *ret; + virDomainVcpuPinDefPtr *ret = NULL;
if (VIR_ALLOC_N(ret, nvcpupin) < 0) { goto no_memory; @@ -1514,11 +1514,15 @@ virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin) return ret;
no_memory: - while (i >= 0) { - VIR_FREE(ret[i]->cpumask); - VIR_FREE(ret[i]); + if (ret) { + for ( ; i >= 0; --i) { + if (ret[i]) { + VIR_FREE(ret[i]->cpumask); + VIR_FREE(ret[i]); + } + } + VIR_FREE(ret); } - VIR_FREE(ret); virReportOOMError();
return NULL;
Whoops, ACK ! Please push BTW I'm unclear what our prefered style is for if (ret) { ... VIR_FREE(ret); } vs if (ret) { ... } VIR_FREE(ret); because that pattern appears twice in the replacement code. To me that's fine, but since we avoid if (ret) VIR_FREE(ret); I'm asking :-) 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/