qemu: Fix handling of passed FDs in remoteDispatchDomainFdAssociate

To ensure same behaviour when remote driver is or is not used we must
not steal the FDs and array holding them passed to qemuDomainFDAssociate
but rather duplicate them. At the same time the remote driver must close
and free them to prevent leak.

Pointed out by Coverity as FD leak on error path:

 *** CID 404348:  Resource leaks  (RESOURCE_LEAK)
 /src/remote/remote_daemon_dispatch.c: 7484 in remoteDispatchDomainFdAssociate()
 7478         rv = 0;
 7479
 7480      cleanup:
 7481         if (rv < 0)
 7482             virNetMessageSaveError(rerr);
 7483         virObjectUnref(dom);
 >>>     CID 404348:  Resource leaks  (RESOURCE_LEAK)
 >>>     Variable "fds" going out of scope leaks the storage it points to.
 7484         return rv;

Fixes: abd9025c2fd
Fixes: f762f87534e
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a88c9ebe642cb9638dc2d60ce0cfc326f2d90b94..d6879175fece63aadc813380d10d2af91e9bc60c 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20442,7 +20442,8 @@ qemuDomainFDAssociate(virDomainPtr domain,
 {
     virDomainObj *vm = NULL;
     qemuDomainObjPrivate *priv;
-    virStorageSourceFDTuple *new;
+    g_autoptr(virStorageSourceFDTuple) new = NULL;
+    size_t i;
     int ret = -1;
 
     virCheckFlags(VIR_DOMAIN_FD_ASSOCIATE_SECLABEL_RESTORE |
@@ -20460,8 +20461,16 @@ qemuDomainFDAssociate(virDomainPtr domain,
     priv = vm->privateData;
 
     new = virStorageSourceFDTupleNew();
-    new->fds = fds;
     new->nfds = nfds;
+    new->fds = g_new0(int, new->nfds);
+    for (i = 0; i < new->nfds; i++) {
+        if ((new->fds[i] = dup(fds[i])) < 0) {
+            virReportSystemError(errno,
+                                 _("failed to duplicate passed fd with index '%zu'"),
+                                 i);
+            goto cleanup;
+        }
+    }
     new->conn = domain->conn;
 
     new->writable = flags & VIR_DOMAIN_FD_ASSOCIATE_SECLABEL_WRITABLE;
@@ -20469,7 +20478,7 @@ qemuDomainFDAssociate(virDomainPtr domain,
 
     virCloseCallbacksDomainAdd(vm, domain->conn, qemuDomainFDHashCloseConnect);
 
-    g_hash_table_insert(priv->fds, g_strdup(name), new);
+    g_hash_table_insert(priv->fds, g_strdup(name), g_steal_pointer(&new));
 
     ret = 0;
 

diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 40c734ce6b1c46a2bd554367c64aab66944468d4..6c56e9ec3e255eeb2d9cd9e655224654cdf0d08c 100644 (file)
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -7478,6 +7478,9 @@ remoteDispatchDomainFdAssociate(virNetServer *server G_GNUC_UNUSED,
     rv = 0;
 
  cleanup:
+    for (i = 0; i < nfds; i++)
+        VIR_FORCE_CLOSE(fds[i]);
+    g_free(fds);
     if (rv < 0)
         virNetMessageSaveError(rerr);
     virObjectUnref(dom);