[PATCH 4/5] vircommand: Introduce virCommandMassCloseRange()

Michal Privoznik mprivozn at redhat.com
Tue Aug 22 13:34:40 UTC 2023


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 at redhat.com>
---
 src/util/vircommand.c | 77 ++++++++++++++++++++++++++++++++++++++++---
 tests/commandtest.c   |  2 ++
 2 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 867f45b57b..5f094c625a 100644
--- 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 688cf59160..aa108ce583 100644
--- 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
-- 
2.41.0



More information about the libvir-list mailing list