vircommand: Introduce virCommandMassCloseRange()

This is brand new way of closing FDs before exec(). We need to
close all FDs except those we want to explicitly pass to avoid
leaking FDs into the child. Historically, we've done this by
either iterating over all opened FDs and closing them one by one
(or preserving them), or by iterating over an FD interval [2 ...
N] and closing them one by one followed by calling closefrom(N +
1). This is a lot of syscalls.

That's why Linux kernel developers introduced new close_from
syscall. It closes all FDs within given range, in a single
syscall. Since we keep list of FDs we want to preserve and pass
to the child process, we can use this syscall to close all FDs
in between. We don't even need to care about opened FDs.

Of course, we have to check whether the syscall is available and
fall back to the old implementation if it isn't.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 867f45b57ba19a87e7b0a55e22c1f5d4205ccb02..5f094c625a4063eafe7cae3fddd3276be340b2c5 100644 (file)
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -527,10 +527,10 @@ virCommandMassCloseGetFDsGeneric(virCommand *cmd G_GNUC_UNUSED,
 # endif /* !__linux__ */
 
 static int
-virCommandMassClose(virCommand *cmd,
-                    int childin,
-                    int childout,
-                    int childerr)
+virCommandMassCloseFrom(virCommand *cmd,
+                        int childin,
+                        int childout,
+                        int childerr)
 {
     g_autoptr(virBitmap) fds = NULL;
     int openmax = sysconf(_SC_OPEN_MAX);
@@ -597,6 +597,75 @@ virCommandMassClose(virCommand *cmd,
 }
 
 
+static int
+virCommandMassCloseRange(virCommand *cmd,
+                         int childin,
+                         int childout,
+                         int childerr)
+{
+    g_autoptr(virBitmap) fds = virBitmapNew(0);
+    ssize_t first;
+    ssize_t last;
+    size_t i;
+
+    virBitmapSetBitExpand(fds, childin);
+    virBitmapSetBitExpand(fds, childout);
+    virBitmapSetBitExpand(fds, childerr);
+
+    for (i = 0; i < cmd->npassfd; i++) {
+        int fd = cmd->passfd[i].fd;
+
+        virBitmapSetBitExpand(fds, fd);
+
+        if (virSetInherit(fd, true) < 0) {
+            virReportSystemError(errno, _("failed to preserve fd %1$d"), fd);
+            return -1;
+        }
+    }
+
+    first = 2;
+    while ((last = virBitmapNextSetBit(fds, first)) >= 0) {
+        if (first + 1 == last) {
+            first = last;
+            continue;
+        }
+
+        /* Preserve @first and @last and close everything in between. */
+        if (virCloseRange(first + 1, last - 1) < 0) {
+            virReportSystemError(errno,
+                                 _("Unable to mass close FDs (first=%1$zd, last=%2$zd)"),
+                                 first + 1, last - 1);
+            return -1;
+        }
+
+        first = last;
+    }
+
+    if (virCloseRange(first + 1, ~0U) < 0) {
+        virReportSystemError(errno,
+                             _("Unable to mass close FDs (first=%1$zd, last=%2$d"),
+                             first + 1, ~0U);
+        return -1;
+    }
+
+    return 0;
+}
+
+
+
+static int
+virCommandMassClose(virCommand *cmd,
+                    int childin,
+                    int childout,
+                    int childerr)
+{
+    if (virCloseRangeIsSupported())
+        return virCommandMassCloseRange(cmd, childin, childout, childerr);
+
+    return virCommandMassCloseFrom(cmd, childin, childout, childerr);
+}
+
+
 /*
  * virExec:
  * @cmd virCommand * containing all information about the program to

diff --git a/tests/commandtest.c b/tests/commandtest.c
index 688cf59160ced4206f56d8f3c9058806a59ca91e..aa108ce583db494683f4c991d1832447c725bd78 100644 (file)
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1247,6 +1247,8 @@ mymain(void)
     setpgid(0, 0);
     ignore_value(setsid());
 
+    virCloseRangeInit();
+
     /* Our test expects particular fd values; to get that, we must not
      * leak fds that we inherited from a lazy parent.  At the same
      * time, virInitialize may open some fds (perhaps via third-party