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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list