[PATCH v4 1/2] rpc: When adding srv to dmn servers, need to add ref

[John Ferlan <jferlan@xxxxxxxxxx>]

From: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>

Commit id '252610f7d' modified net server management to use a
hash table to store/manage the various servers; however, during
virNetDaemonAddServerPostExec an @srv object is created, added
to the dmn->servers hash table, but did not increment the object
refcnt like was done during virNetDaemonAddServer as if @srv
were being created for the first time.

Without the proper ref a subsequent virObjectUnref done during
virNetDaemonDispose when virHashFree calls the hash table entry
@dataFree function virObjectFreeHashData could inadvertently free
the memory before some caller is really done with it or cause the
caller to attempt to free something that's already been freed (and
it no longer necessarily owns).

Additionally, since commit id 'f08b1c58f3' removed the @srv
from virLockManager in favor of using virNetDaemonGetServer
which creates a ref on @srv, we need to modify the lock_manager
code in order to properly handle and dispose the references
to the @srv object allowing either the cleanup processing or
the virNetDaemonDispose processing to remove the last ref to
the object.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/locking/lock_daemon.c | 19 ++++++++++++++-----
 src/rpc/virnetdaemon.c    |  2 ++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index fe3eaf9032..ee2c35fdb8 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -1312,14 +1312,14 @@ int main(int argc, char **argv) {
         srv = virNetDaemonGetServer(lockDaemon->dmn, "virtlockd");
         if ((rv = virLockDaemonSetupNetworkingSystemD(srv)) < 0) {
             ret = VIR_LOCK_DAEMON_ERR_NETWORK;
-            goto cleanup;
+            goto cleanup_srvref;
         }
 
         /* Only do this, if systemd did not pass a FD */
         if (rv == 0 &&
             virLockDaemonSetupNetworkingNative(srv, sock_file) < 0) {
             ret = VIR_LOCK_DAEMON_ERR_NETWORK;
-            goto cleanup;
+            goto cleanup_srvref;
         }
     } else if (rv == 1) {
         srv = virNetDaemonGetServer(lockDaemon->dmn, "virtlockd");
@@ -1333,7 +1333,7 @@ int main(int argc, char **argv) {
 
     if ((virLockDaemonSetupSignals(lockDaemon->dmn)) < 0) {
         ret = VIR_LOCK_DAEMON_ERR_SIGNAL;
-        goto cleanup;
+        goto cleanup_srvref;
     }
 
     if (!(lockProgram = virNetServerProgramNew(VIR_LOCK_SPACE_PROTOCOL_PROGRAM,
@@ -1341,14 +1341,17 @@ int main(int argc, char **argv) {
                                                virLockSpaceProtocolProcs,
                                                virLockSpaceProtocolNProcs))) {
         ret = VIR_LOCK_DAEMON_ERR_INIT;
-        goto cleanup;
+        goto cleanup_srvref;
     }
 
     if (virNetServerAddProgram(srv, lockProgram) < 0) {
         ret = VIR_LOCK_DAEMON_ERR_INIT;
-        goto cleanup;
+        goto cleanup_srvref;
     }
 
+    /* Completed srv usage from virNetDaemonGetServer */
+    virObjectUnref(srv);
+
     /* Disable error func, now logging is setup */
     virSetErrorFunc(NULL, virLockDaemonErrorHandler);
 
@@ -1403,4 +1406,10 @@ int main(int argc, char **argv) {
  no_memory:
     VIR_ERROR(_("Can't allocate memory"));
     exit(EXIT_FAILURE);
+
+ cleanup_srvref:
+    /* Unref for virNetDaemonGetServer ref - still have "our" ref from
+     * allocation and possibly a ref for being in netserver hash table */
+    virObjectUnref(srv);
+    goto cleanup;
 }
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index e3b9390af2..d970c47ad4 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -313,6 +313,8 @@ virNetDaemonAddServerPostExec(virNetDaemonPtr dmn,
     if (virHashAddEntry(dmn->servers, serverName, srv) < 0)
         goto error;
 
+    virObjectRef(srv);
+
     virJSONValueFree(object);
     virObjectUnlock(dmn);
     return srv;
-- 
2.13.6

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list