From 44a54eb073d2ae52ebf1661ae73bb1f0f98599f9 Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@suse.com>
Date: Wed, 15 Jul 2015 16:35:50 -0600
Subject: [PATCH] libxl: fix ref counting of libxlMigrationDstArgs

This patch fixes some flawed logic around ref counting the
libxlMigrationDstArgs object.

First, when adding sockets to the event loop with
virNetSocketAddIOCallback(), the generic virObjectFreeCallback()
was registered as a free function, with libxlMigrationDstArgs as
its parameter. A reference was also taken on
libxlMigrationDstArgs for each successful call to
virNetSocketAddIOCallback(). The rational behind this logic was
that the libxlMigrationDstArgs object had to out-live the socket
objects. But virNetSocketAddIOCallback() already takes a
reference on socket objects, ensuring their life until removed
from the event loop and unref'ed in virNetSocketEventFree(). We
only need to ensure libxlMigrationDstArgs lives until
libxlDoMigrateReceive() finishes, which can be done by simply
unref'ing libxlMigrationDstArgs at the end of
libxlDoMigrateReceive().

The second flaw was unref'ing the sockets in the failure path of
libxlMigrateReceive() and at the end of libxlDoMigrateReceive().
As mentioned above, the sockets are already unref'ed by
virNetSocketEventFree() when removed from the event loop.
Attempting to unref the socket a second time resulted in a
libvirtd crash since the socket was previously unref'ed and
disposed.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_migration.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index aa9547b492..536e91e7f6 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -109,7 +109,6 @@ libxlDoMigrateReceive(void *opaque)
 
     /* Remove all listen socks from event handler, and close them. */
     for (i = 0; i < nsocks; i++) {
-        virNetSocketUpdateIOCallback(socks[i], 0);
         virNetSocketRemoveIOCallback(socks[i]);
         virNetSocketClose(socks[i]);
         virObjectUnref(socks[i]);
@@ -117,6 +116,7 @@ libxlDoMigrateReceive(void *opaque)
     }
     args->nsocks = 0;
     VIR_FORCE_CLOSE(recvfd);
+    virObjectUnref(args);
 }
 
 
@@ -164,11 +164,11 @@ libxlMigrateReceive(virNetSocketPtr sock,
         virNetSocketUpdateIOCallback(socks[i], 0);
         virNetSocketRemoveIOCallback(socks[i]);
         virNetSocketClose(socks[i]);
-        virObjectUnref(socks[i]);
         socks[i] = NULL;
     }
     args->nsocks = 0;
     VIR_FORCE_CLOSE(recvfd);
+    virObjectUnref(args);
 }
 
 static int
@@ -318,7 +318,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
     virNetSocketPtr *socks = NULL;
     size_t nsocks = 0;
     int nsocks_listen = 0;
-    libxlMigrationDstArgs *args;
+    libxlMigrationDstArgs *args = NULL;
     size_t i;
     int ret = -1;
 
@@ -420,22 +420,12 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
                                       VIR_EVENT_HANDLE_READABLE,
                                       libxlMigrateReceive,
                                       args,
-                                      virObjectFreeCallback) < 0)
+                                      NULL) < 0)
             continue;
 
-        /*
-         * Successfully added sock to event loop.  Take a ref on args to
-         * ensure it is not freed until sock is removed from the event loop.
-         * Ref is dropped in virObjectFreeCallback after being removed
-         * from the event loop.
-         */
-        virObjectRef(args);
         nsocks_listen++;
     }
 
-    /* Done with args in this function, drop reference */
-    virObjectUnref(args);
-
     if (!nsocks_listen)
         goto error;
 
@@ -448,6 +438,8 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
         virObjectUnref(socks[i]);
     }
     VIR_FREE(socks);
+    virObjectUnref(args);
+
     /* Remove virDomainObj from domain list */
     if (vm) {
         virDomainObjListRemove(driver->domains, vm);
-- 
GitLab