As of commit v5.9-rc1~160^2~3 the Linux kernel has close_range()
syscall, which closes not just one FD but whole range. Then, in
its commit glibc-2.34~115 glibc introduced closefrom() which is
just a wrapper over close_range(), but it allows us to use
FreeBSD-only implementation on Linux too, as both OS-es now have
the same function.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 meson.build           |   1 +
 src/util/vircommand.c | 124 ++++++++++++++++++++++--------------------
 2 files changed, 66 insertions(+), 59 deletions(-)

diff --git a/meson.build b/meson.build
index aa391e7178..a4b52b6156 100644
--- a/meson.build
+++ b/meson.build
@@ -573,6 +573,7 @@ libvirt_export_dynamic = cc.first_supported_link_argument([
 # check availability of various common functions (non-fatal if missing)
 
 functions = [
+  'closefrom',
   'elf_aux_info',
   'explicit_bzero',
   'fallocate',
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 49abb53c28..b8b8d48f92 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -479,7 +479,68 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups)
     return 0;
 }
 
-# ifdef __linux__
+# ifdef WITH_CLOSEFROM
+#  define USE_CLOSEFROM
+# else
+#  define USE_GENERIC
+# endif
+
+
+# ifdef USE_CLOSEFROM
+static int
+virCommandMassClose(virCommand *cmd,
+                    int childin,
+                    int childout,
+                    int childerr)
+{
+    int lastfd = -1;
+    int fd = -1;
+    size_t i;
+
+    /*
+     * Two phases of closing.
+     *
+     * The first (inefficient) phase iterates over FDs,
+     * preserving certain FDs we need to pass down, and
+     * closing others. The number of iterations is bounded
+     * to the number of the biggest FD we need to preserve.
+     *
+     * The second (speedy) phase uses closefrom() to cull
+     * all remaining FDs in the process.
+     *
+     * Usually the first phase will be fairly quick only
+     * processing a handful of low FD numbers, and thus using
+     * closefrom() is a massive win for high ulimit() NFILES
+     * values.
+     */
+    lastfd = MAX(lastfd, childin);
+    lastfd = MAX(lastfd, childout);
+    lastfd = MAX(lastfd, childerr);
+
+    for (i = 0; i < cmd->npassfd; i++)
+        lastfd = MAX(lastfd, cmd->passfd[i].fd);
+
+    for (fd = 0; fd <= lastfd; fd++) {
+        if (fd == childin || fd == childout || fd == childerr)
+            continue;
+        if (!virCommandFDIsSet(cmd, fd)) {
+            int tmpfd = fd;
+            VIR_MASS_CLOSE(tmpfd);
+        } else if (virSetInherit(fd, true) < 0) {
+            virReportSystemError(errno, _("failed to preserve fd %1$d"), fd);
+            return -1;
+        }
+    }
+
+    closefrom(lastfd + 1);
+
+    return 0;
+}
+# endif /* ! WITH_CLOSEFROM */
+
+
+# ifdef USE_GENERIC
+#  ifdef __linux__
 /* On Linux, we can utilize procfs and read the table of opened
  * FDs and selectively close only those FDs we don't want to pass
  * onto child process (well, the one we will exec soon since this
@@ -515,7 +576,7 @@ virCommandMassCloseGetFDsLinux(virCommand *cmd G_GNUC_UNUSED,
     return 0;
 }
 
-# else /* !__linux__ */
+#  else /* !__linux__ */
 
 static int
 virCommandMassCloseGetFDsGeneric(virCommand *cmd G_GNUC_UNUSED,
@@ -524,61 +585,7 @@ virCommandMassCloseGetFDsGeneric(virCommand *cmd G_GNUC_UNUSED,
     virBitmapSetAll(fds);
     return 0;
 }
-# endif /* !__linux__ */
-
-# ifdef __FreeBSD__
-
-static int
-virCommandMassClose(virCommand *cmd,
-                    int childin,
-                    int childout,
-                    int childerr)
-{
-    int lastfd = -1;
-    int fd = -1;
-    size_t i;
-
-    /*
-     * Two phases of closing.
-     *
-     * The first (inefficient) phase iterates over FDs,
-     * preserving certain FDs we need to pass down, and
-     * closing others. The number of iterations is bounded
-     * to the number of the biggest FD we need to preserve.
-     *
-     * The second (speedy) phase uses closefrom() to cull
-     * all remaining FDs in the process.
-     *
-     * Usually the first phase will be fairly quick only
-     * processing a handful of low FD numbers, and thus using
-     * closefrom() is a massive win for high ulimit() NFILES
-     * values.
-     */
-    lastfd = MAX(lastfd, childin);
-    lastfd = MAX(lastfd, childout);
-    lastfd = MAX(lastfd, childerr);
-
-    for (i = 0; i < cmd->npassfd; i++)
-        lastfd = MAX(lastfd, cmd->passfd[i].fd);
-
-    for (fd = 0; fd <= lastfd; fd++) {
-        if (fd == childin || fd == childout || fd == childerr)
-            continue;
-        if (!virCommandFDIsSet(cmd, fd)) {
-            int tmpfd = fd;
-            VIR_MASS_CLOSE(tmpfd);
-        } else if (virSetInherit(fd, true) < 0) {
-            virReportSystemError(errno, _("failed to preserve fd %1$d"), fd);
-            return -1;
-        }
-    }
-
-    closefrom(lastfd + 1);
-
-    return 0;
-}
-
-# else /* ! __FreeBSD__ */
+#  endif /* !__linux__ */
 
 static int
 virCommandMassClose(virCommand *cmd,
@@ -628,8 +635,7 @@ virCommandMassClose(virCommand *cmd,
 
     return 0;
 }
-
-# endif /* ! __FreeBSD__ */
+# endif /* ! USE_GENERIC */
 
 
 /*
-- 
2.39.3

On Wed, Jun 21, 2023 at 04:09:09PM +0200, Michal Privoznik wrote:
> As of commit v5.9-rc1~160^2~3 the Linux kernel has close_range()
> syscall, which closes not just one FD but whole range. Then, in
> its commit glibc-2.34~115 glibc introduced closefrom() which is
> just a wrapper over close_range(), but it allows us to use
> FreeBSD-only implementation on Linux too, as both OS-es now have
> the same function.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  meson.build           |   1 +
>  src/util/vircommand.c | 124 ++++++++++++++++++++++--------------------
>  2 files changed, 66 insertions(+), 59 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index aa391e7178..a4b52b6156 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -573,6 +573,7 @@ libvirt_export_dynamic = cc.first_supported_link_argument([
>  # check availability of various common functions (non-fatal if missing)
>  
>  functions = [
> +  'closefrom',
>    'elf_aux_info',
>    'explicit_bzero',
>    'fallocate',
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index 49abb53c28..b8b8d48f92 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -479,7 +479,68 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups)
>      return 0;
>  }
>  
> -# ifdef __linux__
> +# ifdef WITH_CLOSEFROM
> +#  define USE_CLOSEFROM
> +# else
> +#  define USE_GENERIC
> +# endif
> +
> +
> +# ifdef USE_CLOSEFROM
> +static int
> +virCommandMassClose(virCommand *cmd,
> +                    int childin,
> +                    int childout,
> +                    int childerr)
> +{
> +    int lastfd = -1;
> +    int fd = -1;
> +    size_t i;
> +
> +    /*
> +     * Two phases of closing.
> +     *
> +     * The first (inefficient) phase iterates over FDs,
> +     * preserving certain FDs we need to pass down, and
> +     * closing others. The number of iterations is bounded
> +     * to the number of the biggest FD we need to preserve.
> +     *
> +     * The second (speedy) phase uses closefrom() to cull
> +     * all remaining FDs in the process.
> +     *
> +     * Usually the first phase will be fairly quick only
> +     * processing a handful of low FD numbers, and thus using
> +     * closefrom() is a massive win for high ulimit() NFILES
> +     * values.
> +     */
> +    lastfd = MAX(lastfd, childin);
> +    lastfd = MAX(lastfd, childout);
> +    lastfd = MAX(lastfd, childerr);
> +
> +    for (i = 0; i < cmd->npassfd; i++)
> +        lastfd = MAX(lastfd, cmd->passfd[i].fd);
> +
> +    for (fd = 0; fd <= lastfd; fd++) {
> +        if (fd == childin || fd == childout || fd == childerr)
> +            continue;
> +        if (!virCommandFDIsSet(cmd, fd)) {
> +            int tmpfd = fd;
> +            VIR_MASS_CLOSE(tmpfd);
> +        } else if (virSetInherit(fd, true) < 0) {
> +            virReportSystemError(errno, _("failed to preserve fd %1$d"), fd);
> +            return -1;
> +        }
> +    }
> +
> +    closefrom(lastfd + 1);

GLibC might have closefrom() but be in a container running on an
older kernel that lacks close_range syscall.

We could ignore return value on FreeBSD, but now on Linux we must
check the return value and fallback to the old codepath(s).


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|