[PATCH v2 3/3] policycoreutils/setfiles: honor RESTORECON_THREADS

jboero <[email protected]>
Newsgroups org.kernel.vger.selinux
Message-ID <[email protected]>
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.  parse_nthreads() does not touch nthreads
on error, so whichever default is compiled in stays in effect.

Link: https://github.com/SELinuxProject/selinux/issues/489
Signed-off-by: Johnny Boero <[email protected]>
---

This replaces 3/3 of the v1 series; 1/3 and 2/3 are unchanged and still
apply as sent.

v2: drop the "nthreads = 0" reset on an invalid value, as Stephen
    Smalley pointed out.  parse_nthreads() does not write nthreads on error, so the
    reset was a no-op with 2/3 applied and turned an invalid value into
    all-cores without it.  Verified by building 1/3 + 3/3 without 2/3:
    before the fix an invalid value ran at ~1000% CPU, after it stays
    single threaded at 99%.  Commit message reworded accordingly.

 policycoreutils/setfiles/restorecon.8 | 17 ++++++++++++++++-
 policycoreutils/setfiles/setfiles.8   | 18 +++++++++++++++++-
 policycoreutils/setfiles/setfiles.c   | 18 ++++++++++++++++--
 3 files changed, 49 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..7f1c0a31 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,19 @@ 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;
+	 * parse_nthreads() leaves nthreads alone in that case.
+	 */
+	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);
+
 	/* Process any options. */
 	while ((opt = getopt(argc, argv, opts)) > 0) {
 		switch (opt) {
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.