[PATCH 1/3] vircommand: Use closefrom() more often

[Daniel P. Berrangé]

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 at 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 :|