Re: [PATCH v1 3/3] policycoreutils/setfiles: honor RESTORECON_THREADS

On Tue, Aug 18, 2026 at 3:24 PM jboero <[email protected]> wrote:
>
> From: Johnny Boero <[email protected]>
>
> Relabeling is invoked from places that cannot reasonably be given a -T
> option.  On a Fedora 44 system, 10 installed packages run restorecon
> from their scriptlets -- filesystem, kernel-core, container-selinux and
> selinux-policy-targeted among them -- and not one of them passes -T:
>
>   restorecon -R /usr/bin /usr/sbin
>   restorecon -e /run/media -R /root /var/log /var/run /etc/passwd* ...
>   restorecon -R /var/lib/containers
>
> Influencing those means patching every spec file in the distribution,
> one merge request at a time, and only covers the packages one happens to
> have installed.  The environment is the only knob that reaches them all
> at once.
>
> Add RESTORECON_THREADS, with the same meaning as -T, which takes
> precedence when both are given:
>
>   RESTORECON_THREADS=1 dnf update  # keep relabeling on a single core
>   RESTORECON_THREADS=8 dnf update  # cap parallelism during application load
>
> An invalid value is reported and ignored rather than being fatal, since
> the variable is inherited by every child process and should not be able
> to break an unrelated caller.
>
> This patch stands on its own: it is the brake for the new default in the
> preceding patch, and equally the accelerator if the default is left at a
> single thread.

It actually depends on patch 2/3, see below.

>
> Link: https://github.com/SELinuxProject/selinux/issues/489
> Signed-off-by: Johnny Boero <[email protected]>
> ---
>
> diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
> index 4c860755..ee0930b7 100644
> --- a/policycoreutils/setfiles/setfiles.c
> +++ b/policycoreutils/setfiles/setfiles.c
> @@ -65,8 +65,8 @@ static void set_rootpath(const char *arg)
>  }
>
>  /*
> - * Parse a thread count, as given by the -T option.  Returns -1 on invalid
> - * input.
> + * Parse a thread count, as given by the -T option or the
> + * RESTORECON_THREADS environment variable.  Returns -1 on invalid input.
>   */
>  static int parse_nthreads(const char *str, size_t *nthreads)
>  {
> @@ -166,6 +166,7 @@ int main(int argc, char **argv)
>         struct stat sb;
>         int opt, i = 0;
>         const char *input_filename = NULL;
> +       const char *env_nthreads;
>         int use_input_file = 0;
>         char *buf = NULL;
>         size_t buf_len = 0, nthreads = 0;
> @@ -246,6 +247,20 @@ int main(int argc, char **argv)
>                         exit(0);
>         }
>
> +       /*
> +        * An explicit -T option takes precedence over the environment.  An
> +        * invalid value is not fatal, so that a bogus setting inherited by
> +        * every child process does not break unrelated callers.
> +        */
> +       env_nthreads = getenv("RESTORECON_THREADS");
> +       if (env_nthreads && env_nthreads[0] != '\0' &&
> +           parse_nthreads(env_nthreads, &nthreads) < 0) {
> +               fprintf(stderr,
> +                       "%s:  invalid RESTORECON_THREADS value \"%s\", ignoring\n",
> +                       r_opts.progname, env_nthreads);
> +               nthreads = 0;

parse_nthreads() doesn't write nthreads on error so nthreads always
retains the compiled default.
With patch 2 applied first, that default is 0 and this line is a
no-op; without patch 2, an invalid RESTORECON_THREADS would flip the
default from 1 to all-cores instead of being ignored.
Drop this line and re-submit please.

> +       }
> +
>         /* Process any options. */
>         while ((opt = getopt(argc, argv, opts)) > 0) {
>                 switch (opt) {
> --
> 2.55.0
>