[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] qemu: Fix possible infinite loop and segfault on error path.



On 08/30/12 16:06, Daniel Veillard wrote:
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);

This alternative saves the one function call and the identical check, so I tend to use it rather than the other one

}

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


Pushed. Thanks!

Peter


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]