[PATCH 2/4] virNetSocket: Be more safe with fork() around virNetSocketDupFD()

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

If there was a caller which would dup the client FD without
CLOEXEC flag and later decided to change the flag it wouldn't be
safe to do because fork() might have had occurred meantime.
Switch to the other pattern - always dup FD with the flag set and
let callers clear the flag if they need to do so.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/libxl/libxl_migration.c |  4 ++--
 src/qemu/qemu_migration.c   |  2 +-
 src/rpc/virnetclient.c      | 10 +++++++++-
 src/rpc/virnetsocket.c      |  7 ++-----
 src/rpc/virnetsocket.h      |  2 +-
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index fc7ccb53d0..5eb8eb1203 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -310,7 +310,7 @@ libxlMigrateDstReceive(virNetSocketPtr sock,
     }
     VIR_DEBUG("Accepted migration connection."
               "  Spawning thread to process migration data");
-    recvfd = virNetSocketDupFD(client_sock, true);
+    recvfd = virNetSocketDupFD(client_sock);
     virObjectUnref(client_sock);
 
     /*
@@ -1254,7 +1254,7 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivatePtr driver,
         goto cleanup;
     }
 
-    sockfd = virNetSocketDupFD(sock, true);
+    sockfd = virNetSocketDupFD(sock);
     virObjectUnref(sock);
 
     if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 825a9d399b..129be0a11a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3303,7 +3303,7 @@ qemuMigrationSrcConnect(virQEMUDriverPtr driver,
     if (virNetSocketNewConnectTCP(host, port,
                                   AF_UNSPEC,
                                   &sock) == 0) {
-        spec->dest.fd.qemu = virNetSocketDupFD(sock, true);
+        spec->dest.fd.qemu = virNetSocketDupFD(sock);
         virObjectUnref(sock);
     }
     if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0 ||
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index b4d8fb2187..40ed3fde6d 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -715,8 +715,16 @@ int virNetClientGetFD(virNetClientPtr client)
 int virNetClientDupFD(virNetClientPtr client, bool cloexec)
 {
     int fd;
+
     virObjectLock(client);
-    fd = virNetSocketDupFD(client->sock, cloexec);
+
+    fd = virNetSocketDupFD(client->sock);
+    if (!cloexec && fd >= 0 && virSetInherit(fd, true)) {
+        virReportSystemError(errno, "%s",
+                             _("Cannot disable close-on-exec flag"));
+        VIR_FORCE_CLOSE(fd);
+    }
+
     virObjectUnlock(client);
     return fd;
 }
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 55de3b2aad..27ffa23bcd 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1372,14 +1372,11 @@ int virNetSocketGetFD(virNetSocketPtr sock)
 }
 
 
-int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec)
+int virNetSocketDupFD(virNetSocketPtr sock)
 {
     int fd;
 
-    if (cloexec)
-        fd = fcntl(sock->fd, F_DUPFD_CLOEXEC, 0);
-    else
-        fd = dup(sock->fd);
+    fd = fcntl(sock->fd, F_DUPFD_CLOEXEC, 0);
     if (fd < 0) {
         virReportSystemError(errno, "%s",
                              _("Unable to copy socket file handle"));
diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h
index de795af917..e6bac27566 100644
--- a/src/rpc/virnetsocket.h
+++ b/src/rpc/virnetsocket.h
@@ -124,7 +124,7 @@ virNetSocketPtr virNetSocketNewPostExecRestart(virJSONValuePtr object);
 virJSONValuePtr virNetSocketPreExecRestart(virNetSocketPtr sock);
 
 int virNetSocketGetFD(virNetSocketPtr sock);
-int virNetSocketDupFD(virNetSocketPtr sock, bool cloexec);
+int virNetSocketDupFD(virNetSocketPtr sock);
 
 bool virNetSocketIsLocal(virNetSocketPtr sock);
 
-- 
2.16.4

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