* [PATCH v1 1/3] policycoreutils/setfiles: reject invalid -T arguments
2026-08-18 19:23 [PATCH v1 0/3] setfiles: control relabeling parallelism from the environment jboero
@ 2026-08-18 19:23 ` jboero
2026-08-18 20:04 ` Stephen Smalley
2026-08-18 19:23 ` [PATCH v1 2/3] policycoreutils/setfiles: use all CPU cores by default jboero
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: jboero @ 2026-08-18 19:23 UTC (permalink / raw)
To: selinux; +Cc: stephen.smalley.work, cgzones, Johnny Boero
From: Johnny Boero <boeroboy@gmail.com>
The -T option is parsed with strtoull(3), which happily accepts leading
whitespace, a sign and, in particular, negative values: "-T -1" wraps
around to SIZE_MAX threads, which then fails a calloc(3) of SIZE_MAX
pthread_t entries rather than reporting a usage error.
Factor the parsing out into a helper and reject anything that is not a
plain decimal thread count.
Signed-off-by: Johnny Boero <boeroboy@gmail.com>
---
policycoreutils/setfiles/setfiles.c | 31 ++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
index da5d2024..c86cc0c9 100644
--- a/policycoreutils/setfiles/setfiles.c
+++ b/policycoreutils/setfiles/setfiles.c
@@ -64,6 +64,32 @@ static void set_rootpath(const char *arg)
}
}
+/*
+ * Parse a thread count, as given by the -T option. Returns -1 on invalid
+ * input.
+ */
+static int parse_nthreads(const char *str, size_t *nthreads)
+{
+ unsigned long long value;
+ char *endptr;
+
+ /*
+ * Reject anything strtoull(3) would silently accept but that is not
+ * a plain thread count, most notably negative values, which would
+ * otherwise wrap around to a huge number of threads.
+ */
+ if (!isdigit((unsigned char)*str))
+ return -1;
+
+ errno = 0;
+ value = strtoull(str, &endptr, 10);
+ if (errno != 0 || *endptr != '\0' || value > SIZE_MAX)
+ return -1;
+
+ *nthreads = value;
+ return 0;
+}
+
static int canoncon(char **contextp)
{
char *context = *contextp, *tmpcon;
@@ -141,7 +167,7 @@ int main(int argc, char **argv)
int opt, i = 0;
const char *input_filename = NULL;
int use_input_file = 0;
- char *buf = NULL, *endptr;
+ char *buf = NULL;
size_t buf_len = 0, nthreads = 1;
const char *base;
int errors = 0;
@@ -376,8 +402,7 @@ int main(int argc, char **argv)
r_opts.xdev = SELINUX_RESTORECON_XDEV;
break;
case 'T':
- nthreads = strtoull(optarg, &endptr, 10);
- if (*optarg == '\0' || *endptr != '\0')
+ if (parse_nthreads(optarg, &nthreads) < 0)
usage(argv[0]);
break;
case 'A':
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1 1/3] policycoreutils/setfiles: reject invalid -T arguments
2026-08-18 19:23 ` [PATCH v1 1/3] policycoreutils/setfiles: reject invalid -T arguments jboero
@ 2026-08-18 20:04 ` Stephen Smalley
2026-08-19 16:41 ` James Carter
0 siblings, 1 reply; 10+ messages in thread
From: Stephen Smalley @ 2026-08-18 20:04 UTC (permalink / raw)
To: jboero; +Cc: selinux, cgzones
On Tue, Aug 18, 2026 at 3:24 PM jboero <boeroboy@gmail.com> wrote:
>
> From: Johnny Boero <boeroboy@gmail.com>
>
> The -T option is parsed with strtoull(3), which happily accepts leading
> whitespace, a sign and, in particular, negative values: "-T -1" wraps
> around to SIZE_MAX threads, which then fails a calloc(3) of SIZE_MAX
> pthread_t entries rather than reporting a usage error.
>
> Factor the parsing out into a helper and reject anything that is not a
> plain decimal thread count.
>
> Signed-off-by: Johnny Boero <boeroboy@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/3] policycoreutils/setfiles: reject invalid -T arguments
2026-08-18 20:04 ` Stephen Smalley
@ 2026-08-19 16:41 ` James Carter
2026-08-19 16:42 ` John Boero
0 siblings, 1 reply; 10+ messages in thread
From: James Carter @ 2026-08-19 16:41 UTC (permalink / raw)
To: Stephen Smalley; +Cc: jboero, selinux, cgzones
On Tue, Aug 18, 2026 at 4:05 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Tue, Aug 18, 2026 at 3:24 PM jboero <boeroboy@gmail.com> wrote:
> >
> > From: Johnny Boero <boeroboy@gmail.com>
> >
> > The -T option is parsed with strtoull(3), which happily accepts leading
> > whitespace, a sign and, in particular, negative values: "-T -1" wraps
> > around to SIZE_MAX threads, which then fails a calloc(3) of SIZE_MAX
> > pthread_t entries rather than reporting a usage error.
> >
> > Factor the parsing out into a helper and reject anything that is not a
> > plain decimal thread count.
> >
> > Signed-off-by: Johnny Boero <boeroboy@gmail.com>
>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
This series has been merged.
Thanks,
Jim
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/3] policycoreutils/setfiles: reject invalid -T arguments
2026-08-19 16:41 ` James Carter
@ 2026-08-19 16:42 ` John Boero
0 siblings, 0 replies; 10+ messages in thread
From: John Boero @ 2026-08-19 16:42 UTC (permalink / raw)
To: James Carter; +Cc: Stephen Smalley, selinux, cgzones
Wow thank you. Very helpful. I'll set this in my global profile now.
On Wed, Aug 19, 2026 at 11:41 AM James Carter <jwcart2@gmail.com> wrote:
>
> On Tue, Aug 18, 2026 at 4:05 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Tue, Aug 18, 2026 at 3:24 PM jboero <boeroboy@gmail.com> wrote:
> > >
> > > From: Johnny Boero <boeroboy@gmail.com>
> > >
> > > The -T option is parsed with strtoull(3), which happily accepts leading
> > > whitespace, a sign and, in particular, negative values: "-T -1" wraps
> > > around to SIZE_MAX threads, which then fails a calloc(3) of SIZE_MAX
> > > pthread_t entries rather than reporting a usage error.
> > >
> > > Factor the parsing out into a helper and reject anything that is not a
> > > plain decimal thread count.
> > >
> > > Signed-off-by: Johnny Boero <boeroboy@gmail.com>
> >
> > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>
> This series has been merged.
> Thanks,
> Jim
>
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 2/3] policycoreutils/setfiles: use all CPU cores by default
2026-08-18 19:23 [PATCH v1 0/3] setfiles: control relabeling parallelism from the environment jboero
2026-08-18 19:23 ` [PATCH v1 1/3] policycoreutils/setfiles: reject invalid -T arguments jboero
@ 2026-08-18 19:23 ` jboero
2026-08-18 20:05 ` Stephen Smalley
2026-08-18 19:23 ` [PATCH v1 3/3] policycoreutils/setfiles: honor RESTORECON_THREADS jboero
2026-08-19 12:57 ` [PATCH v1 0/3] setfiles: control relabeling parallelism from the environment Stephen Smalley
3 siblings, 1 reply; 10+ messages in thread
From: jboero @ 2026-08-18 19:23 UTC (permalink / raw)
To: selinux; +Cc: stephen.smalley.work, cgzones, Johnny Boero
From: Johnny Boero <boeroboy@gmail.com>
setfiles(8) and restorecon(8) have supported parallel relabeling via -T
since commit 93902fc8b0b0 ("setfiles/restorecon: support parallel
relabeling"), but default to a single thread, so anything that does not
pass -T explicitly relabels on one core.
That is most callers. RPM scriptlets, fixfiles(8) and hand-run
restorecon invocations all use the default, and on a machine with a high
core count the result is a long, almost entirely idle relabel: a package
upgrade on an 88 core system spent hours in restorecon with 87 cores
doing nothing.
Default nthreads to 0, i.e. one thread per available CPU core. The
relabeling is already serialized where it needs to be, and -T 1 remains
available for callers that want the previous behaviour.
Relabeling /usr/share (~588k files, dry run) on an 88 core system:
-T 1 20.43s
-T 0 15.78s
Link: https://github.com/SELinuxProject/selinux/issues/489
Signed-off-by: Johnny Boero <boeroboy@gmail.com>
---
policycoreutils/setfiles/restorecon.8 | 2 +-
policycoreutils/setfiles/setfiles.8 | 2 +-
policycoreutils/setfiles/setfiles.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/policycoreutils/setfiles/restorecon.8 b/policycoreutils/setfiles/restorecon.8
index 443f29ab..8d7b46f0 100644
--- a/policycoreutils/setfiles/restorecon.8
+++ b/policycoreutils/setfiles/restorecon.8
@@ -186,7 +186,7 @@ from crossing file system boundaries.
use up to
.I nthreads
threads. Specify 0 to create as many threads as there are available
-CPU cores; 1 to use only a single thread (default); or any positive
+CPU cores (default); 1 to use only a single thread; or any positive
number to use the given number of threads (if possible).
.TP
.SH "ARGUMENTS"
diff --git a/policycoreutils/setfiles/setfiles.8 b/policycoreutils/setfiles/setfiles.8
index b521df22..53cb97cc 100644
--- a/policycoreutils/setfiles/setfiles.8
+++ b/policycoreutils/setfiles/setfiles.8
@@ -191,7 +191,7 @@ produces input suitable for this mode.
use up to
.I nthreads
threads. Specify 0 to create as many threads as there are available
-CPU cores; 1 to use only a single thread (default); or any positive
+CPU cores (default); 1 to use only a single thread; or any positive
number to use the given number of threads (if possible).
.TP
.B \-A
diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
index c86cc0c9..4c860755 100644
--- a/policycoreutils/setfiles/setfiles.c
+++ b/policycoreutils/setfiles/setfiles.c
@@ -168,7 +168,7 @@ int main(int argc, char **argv)
const char *input_filename = NULL;
int use_input_file = 0;
char *buf = NULL;
- size_t buf_len = 0, nthreads = 1;
+ size_t buf_len = 0, nthreads = 0;
const char *base;
int errors = 0;
const char *ropts = "ce:f:hijIDlmno:pqrsvFURW0xT:";
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1 2/3] policycoreutils/setfiles: use all CPU cores by default
2026-08-18 19:23 ` [PATCH v1 2/3] policycoreutils/setfiles: use all CPU cores by default jboero
@ 2026-08-18 20:05 ` Stephen Smalley
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Smalley @ 2026-08-18 20:05 UTC (permalink / raw)
To: jboero; +Cc: selinux, cgzones
On Tue, Aug 18, 2026 at 3:24 PM jboero <boeroboy@gmail.com> wrote:
>
> From: Johnny Boero <boeroboy@gmail.com>
>
> setfiles(8) and restorecon(8) have supported parallel relabeling via -T
> since commit 93902fc8b0b0 ("setfiles/restorecon: support parallel
> relabeling"), but default to a single thread, so anything that does not
> pass -T explicitly relabels on one core.
>
> That is most callers. RPM scriptlets, fixfiles(8) and hand-run
> restorecon invocations all use the default, and on a machine with a high
> core count the result is a long, almost entirely idle relabel: a package
> upgrade on an 88 core system spent hours in restorecon with 87 cores
> doing nothing.
>
> Default nthreads to 0, i.e. one thread per available CPU core. The
> relabeling is already serialized where it needs to be, and -T 1 remains
> available for callers that want the previous behaviour.
>
> Relabeling /usr/share (~588k files, dry run) on an 88 core system:
>
> -T 1 20.43s
> -T 0 15.78s
>
> Link: https://github.com/SELinuxProject/selinux/issues/489
> Signed-off-by: Johnny Boero <boeroboy@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 3/3] policycoreutils/setfiles: honor RESTORECON_THREADS
2026-08-18 19:23 [PATCH v1 0/3] setfiles: control relabeling parallelism from the environment jboero
2026-08-18 19:23 ` [PATCH v1 1/3] policycoreutils/setfiles: reject invalid -T arguments jboero
2026-08-18 19:23 ` [PATCH v1 2/3] policycoreutils/setfiles: use all CPU cores by default jboero
@ 2026-08-18 19:23 ` jboero
2026-08-18 20:10 ` Stephen Smalley
2026-08-19 12:57 ` [PATCH v1 0/3] setfiles: control relabeling parallelism from the environment Stephen Smalley
3 siblings, 1 reply; 10+ messages in thread
From: jboero @ 2026-08-18 19:23 UTC (permalink / raw)
To: selinux; +Cc: stephen.smalley.work, cgzones, Johnny Boero
From: Johnny Boero <boeroboy@gmail.com>
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.
Link: https://github.com/SELinuxProject/selinux/issues/489
Signed-off-by: Johnny Boero <boeroboy@gmail.com>
---
policycoreutils/setfiles/restorecon.8 | 17 ++++++++++++++++-
policycoreutils/setfiles/setfiles.8 | 18 +++++++++++++++++-
policycoreutils/setfiles/setfiles.c | 19 +++++++++++++++++--
3 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/policycoreutils/setfiles/restorecon.8 b/policycoreutils/setfiles/restorecon.8
index 8d7b46f0..001039b7 100644
--- a/policycoreutils/setfiles/restorecon.8
+++ b/policycoreutils/setfiles/restorecon.8
@@ -187,8 +187,23 @@ use up to
.I nthreads
threads. Specify 0 to create as many threads as there are available
CPU cores (default); 1 to use only a single thread; or any positive
-number to use the given number of threads (if possible).
+number to use the given number of threads (if possible). This option
+overrides the
+.B RESTORECON_THREADS
+environment variable.
+.SH "ENVIRONMENT"
.TP
+.B RESTORECON_THREADS
+Sets the default number of threads to use, with the same meaning as the
+.B \-T
+option. This allows the amount of parallelism to be controlled for
+callers that do not pass
+.B \-T
+themselves, such as package installation scripts. Setting it to 1
+restores the historic single threaded behaviour. An invalid value is
+ignored with a warning. The
+.B \-T
+option takes precedence.
.SH "ARGUMENTS"
.IR pathname \ ...
The pathname for the file(s) to be relabeled.
diff --git a/policycoreutils/setfiles/setfiles.8 b/policycoreutils/setfiles/setfiles.8
index 53cb97cc..101556a9 100644
--- a/policycoreutils/setfiles/setfiles.8
+++ b/policycoreutils/setfiles/setfiles.8
@@ -192,12 +192,28 @@ use up to
.I nthreads
threads. Specify 0 to create as many threads as there are available
CPU cores (default); 1 to use only a single thread; or any positive
-number to use the given number of threads (if possible).
+number to use the given number of threads (if possible). This option
+overrides the
+.B RESTORECON_THREADS
+environment variable.
.TP
.B \-A
do not track inodes with multiple hard links or bind mounts that would
match different contexts (saves memory)
+.SH "ENVIRONMENT"
+.TP
+.B RESTORECON_THREADS
+Sets the default number of threads to use, with the same meaning as the
+.B \-T
+option. This allows the amount of parallelism to be controlled for
+callers that do not pass
+.B \-T
+themselves, such as package installation scripts. Setting it to 1
+restores the historic single threaded behaviour. An invalid value is
+ignored with a warning. The
+.B \-T
+option takes precedence.
.SH "ARGUMENTS"
.TP
.I spec_file
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;
+ }
+
/* Process any options. */
while ((opt = getopt(argc, argv, opts)) > 0) {
switch (opt) {
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1 3/3] policycoreutils/setfiles: honor RESTORECON_THREADS
2026-08-18 19:23 ` [PATCH v1 3/3] policycoreutils/setfiles: honor RESTORECON_THREADS jboero
@ 2026-08-18 20:10 ` Stephen Smalley
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Smalley @ 2026-08-18 20:10 UTC (permalink / raw)
To: jboero; +Cc: selinux, cgzones
On Tue, Aug 18, 2026 at 3:24 PM jboero <boeroboy@gmail.com> wrote:
>
> From: Johnny Boero <boeroboy@gmail.com>
>
> 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 <boeroboy@gmail.com>
> ---
>
> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 0/3] setfiles: control relabeling parallelism from the environment
2026-08-18 19:23 [PATCH v1 0/3] setfiles: control relabeling parallelism from the environment jboero
` (2 preceding siblings ...)
2026-08-18 19:23 ` [PATCH v1 3/3] policycoreutils/setfiles: honor RESTORECON_THREADS jboero
@ 2026-08-19 12:57 ` Stephen Smalley
3 siblings, 0 replies; 10+ messages in thread
From: Stephen Smalley @ 2026-08-19 12:57 UTC (permalink / raw)
To: jboero; +Cc: selinux, cgzones
On Tue, Aug 18, 2026 at 3:23 PM jboero <boeroboy@gmail.com> wrote:
>
> From: Johnny Boero <boeroboy@gmail.com>
>
> Hi,
>
> This is the patch for the RFE at
> https://github.com/SELinuxProject/selinux/issues/489, where a package
> upgrade on an 88 core machine spent hours in restorecon with 87 cores
> idle. Stephen Smalley and Christian Göttsche both indicated on the
> issue that switching the default thread count to 0 would be acceptable;
> patch 2 does that, and patch 3 adds the environment variable the RFE
> originally asked for.
>
> Patches 2 and 3 are independent and either can be taken without the
> other.
>
> Patch 1 is a small fix noticed while touching the same code: "-T -1" is
> currently accepted by strtoull(3) and wraps around to SIZE_MAX threads
> instead of producing a usage error.
>
> Patch 2 changes the setfiles(8)/restorecon(8) default from 1 thread to 0,
> i.e. one thread per available CPU core. Only the tools change; the
> selinux_restorecon_parallel(3) default is untouched, so library callers
> are unaffected.
>
> Patch 3 adds RESTORECON_THREADS, with the same meaning as -T, which takes
> precedence. The motivation is reach: relabeling is driven from package
> scriptlets and init scripts that a user cannot edit. On the Fedora 44
> system used for testing, 10 installed packages run restorecon from their
> scriptlets and none of them pass -T, so the environment is the only way
> to influence them without patching every spec file in the distribution.
> An invalid value warns and is ignored rather than being fatal, since the
> variable is inherited by every child process.
>
> Measured with a dry run relabel of /usr/share (~588k files) on an 88 core
> system:
>
> -T 1 / RESTORECON_THREADS=1 ~24s, 99% CPU
> default after this series ~16s, ~1000% CPU
>
> The speedup here is bounded by the directory walk and the syscall mix of
> a dry run; a real relabel that writes contexts benefits more.
>
> Two things I would like feedback on:
>
> - Whether RESTORECON_THREADS is the right name, given the same binary is
> also setfiles(8).
I think that is as good a name as any.
> - Whether an invalid value should warn and continue, as it does here, or
> be fatal.
I think continuing is safer as you noted since this could be inherited
across multiple fork/exec's and
isn't part of the environment that is sanitized by libc. That said,
I'm not sure patch 3 is even
necessary once we take the first two.
>
> Note that this series does not reach libsemanage, which calls
> selinux_restorecon(3) single threaded on the policy store, or rpm's
> selinux plugin, which labels each file inline with lsetfilecon(3) and
> never goes through restorecon at all. Both looked out of scope here.
>
> Johnny Boero (3):
> policycoreutils/setfiles: reject invalid -T arguments
> policycoreutils/setfiles: use all CPU cores by default
> policycoreutils/setfiles: honor RESTORECON_THREADS
>
> policycoreutils/setfiles/restorecon.8 | 19 +++++++++--
> policycoreutils/setfiles/setfiles.8 | 20 +++++++++--
> policycoreutils/setfiles/setfiles.c | 48 ++++++++++++++++++++++++---
> 3 files changed, 79 insertions(+), 8 deletions(-)
>
> --
> 2.55.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread