[RFC PATCH userspace 0/6] Parallel setfiles/restorecon

[RFC PATCH userspace 0/6] Parallel setfiles/restorecon

[Ondrej Mosnacek]

This series adds basic support for parallel relabeling to the libselinux
API and the setfiles/restorecon CLI tools. It turns out that doing the
relabeling in parallel can significantly reduce the time even with a
relatively simple approach.

The first patch is a small cleanup that was found along the way and can
be applied independently. Patches 2-4 are small incremental changes that
make the internal selinux_restorecon functions more thread-safe (I kept
them separate for ease of of review, but maybe they should be rather
folded into the netx patch...). Patch 5 then completes the parallel
relabeling implementation at libselinux level and adds a new function
to the API that allows to make use of it. Finally, patch 6 adds parallel
relabeling support to he setfiles/restorecon tools.

The relevant man pages are also updated to reflect the new
functionality.

The patch descriptions contain more details, namely the last patch has
also some benchmark numbers.

Please test and review. I'm still not fully confident I got everything
right (esp. regarding error handling), but I wanted to put this forward
as an RFC to get some early feedback.

Ondrej Mosnacek (6):
  selinux_restorecon: simplify fl_head allocation by using calloc()
  selinux_restorecon: protect file_spec list with a mutex
  selinux_restorecon: introduce selinux_log_sync()
  selinux_restorecon: add a global mutex to synchronize progress output
  selinux_restorecon: introduce selinux_restorecon_parallel(3)
  setfiles/restorecon: support parallel relabeling

 libselinux/include/selinux/restorecon.h       |  14 +
 libselinux/man/man3/selinux_restorecon.3      |  29 +
 .../man/man3/selinux_restorecon_parallel.3    |   1 +
 libselinux/src/libselinux.map                 |   5 +
 libselinux/src/selinux_internal.h             |  14 +
 libselinux/src/selinux_restorecon.c           | 498 ++++++++++++------
 policycoreutils/setfiles/Makefile             |   2 +-
 policycoreutils/setfiles/restore.c            |   7 +-
 policycoreutils/setfiles/restore.h            |   2 +-
 policycoreutils/setfiles/restorecon.8         |   9 +
 policycoreutils/setfiles/setfiles.8           |   9 +
 policycoreutils/setfiles/setfiles.c           |  28 +-
 12 files changed, 436 insertions(+), 182 deletions(-)
 create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3

-- 
2.30.2

[RFC PATCH userspace 1/6] selinux_restorecon: simplify fl_head allocation by using calloc()

[Ondrej Mosnacek]

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 libselinux/src/selinux_restorecon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 63fb8dc5..62b54079 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -427,10 +427,9 @@ static int filespec_add(ino_t ino, const char *con, const char *file,
 	struct stat64 sb;
 
 	if (!fl_head) {
-		fl_head = malloc(sizeof(file_spec_t) * HASH_BUCKETS);
+		fl_head = calloc(HASH_BUCKETS, sizeof(file_spec_t));
 		if (!fl_head)
 			goto oom;
-		memset(fl_head, 0, sizeof(file_spec_t) * HASH_BUCKETS);
 	}
 
 	h = (ino + (ino >> HASH_BITS)) & HASH_MASK;
-- 
2.30.2

[RFC PATCH userspace 2/6] selinux_restorecon: protect file_spec list with a mutex

[Ondrej Mosnacek]

Not very useful on its own, but will allow to implement a parallel
version of selinux_restorecon() in subsequent patches.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 libselinux/src/selinux_restorecon.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 62b54079..f4a973eb 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -413,6 +413,7 @@ typedef struct file_spec {
 } file_spec_t;
 
 static file_spec_t *fl_head;
+static pthread_mutex_t fl_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 /*
  * Try to add an association between an inode and a context. If there is a
@@ -426,6 +427,8 @@ static int filespec_add(ino_t ino, const char *con, const char *file,
 	int h, ret;
 	struct stat64 sb;
 
+	__pthread_mutex_lock(&fl_mutex);
+
 	if (!fl_head) {
 		fl_head = calloc(HASH_BUCKETS, sizeof(file_spec_t));
 		if (!fl_head)
@@ -446,11 +449,11 @@ static int filespec_add(ino_t ino, const char *con, const char *file,
 				fl->con = strdup(con);
 				if (!fl->con)
 					goto oom;
-				return 1;
+				goto unlock_1;
 			}
 
 			if (strcmp(fl->con, con) == 0)
-				return 1;
+				goto unlock_1;
 
 			selinux_log(SELINUX_ERROR,
 				"conflicting specifications for %s and %s, using %s.\n",
@@ -459,6 +462,9 @@ static int filespec_add(ino_t ino, const char *con, const char *file,
 			fl->file = strdup(file);
 			if (!fl->file)
 				goto oom;
+
+			__pthread_mutex_unlock(&fl_mutex);
+
 			if (flags->conflicterror) {
 				selinux_log(SELINUX_ERROR,
 				"treating conflicting specifications as an error.\n");
@@ -483,13 +489,19 @@ static int filespec_add(ino_t ino, const char *con, const char *file,
 		goto oom_freefl;
 	fl->next = prevfl->next;
 	prevfl->next = fl;
+
+	__pthread_mutex_unlock(&fl_mutex);
 	return 0;
 
 oom_freefl:
 	free(fl);
 oom:
+	__pthread_mutex_unlock(&fl_mutex);
 	selinux_log(SELINUX_ERROR, "%s:  Out of memory\n", __func__);
 	return -1;
+unlock_1:
+	__pthread_mutex_unlock(&fl_mutex);
+	return 1;
 }
 
 /*
-- 
2.30.2

[RFC PATCH userspace 3/6] selinux_restorecon: introduce selinux_log_sync()

[Ondrej Mosnacek]

Add a thread-safe version of selinux_log() and initially use it in
filespec_add(). Together with the previous patch this makes
filespec_add() thread-safe, which will be utilized in subsequent
patches.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 libselinux/src/selinux_restorecon.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index f4a973eb..0ebe56b1 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -46,6 +46,15 @@ static bool selabel_no_digest;
 static char *rootpath = NULL;
 static int rootpathlen;
 
+/* Thread-safe log function for parallel restorecon */
+static pthread_mutex_t log_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+#define selinux_log_sync(type, ...) do { \
+	__pthread_mutex_lock(&log_mutex); \
+	selinux_log(type, __VA_ARGS__); \
+	__pthread_mutex_unlock(&log_mutex); \
+} while(0)
+
 /* Information on excluded fs and directories. */
 struct edir {
 	char *directory;
@@ -455,7 +464,7 @@ static int filespec_add(ino_t ino, const char *con, const char *file,
 			if (strcmp(fl->con, con) == 0)
 				goto unlock_1;
 
-			selinux_log(SELINUX_ERROR,
+			selinux_log_sync(SELINUX_ERROR,
 				"conflicting specifications for %s and %s, using %s.\n",
 				file, fl->file, fl->con);
 			free(fl->file);
@@ -466,7 +475,7 @@ static int filespec_add(ino_t ino, const char *con, const char *file,
 			__pthread_mutex_unlock(&fl_mutex);
 
 			if (flags->conflicterror) {
-				selinux_log(SELINUX_ERROR,
+				selinux_log_sync(SELINUX_ERROR,
 				"treating conflicting specifications as an error.\n");
 				return -1;
 			}
@@ -497,7 +506,7 @@ oom_freefl:
 	free(fl);
 oom:
 	__pthread_mutex_unlock(&fl_mutex);
-	selinux_log(SELINUX_ERROR, "%s:  Out of memory\n", __func__);
+	selinux_log_sync(SELINUX_ERROR, "%s:  Out of memory\n", __func__);
 	return -1;
 unlock_1:
 	__pthread_mutex_unlock(&fl_mutex);
-- 
2.30.2

[RFC PATCH userspace 4/6] selinux_restorecon: add a global mutex to synchronize progress output

[Ondrej Mosnacek]

Another small incremental change to pave the way for a parallel
selinux_restorecon() function.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 libselinux/src/selinux_restorecon.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 0ebe56b1..3f3ac2e6 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -69,6 +69,7 @@ static int exclude_count = 0;
 static struct edir *exclude_lst = NULL;
 static uint64_t fc_count = 0;	/* Number of files processed so far */
 static uint64_t efile_count;	/* Estimated total number of files */
+static pthread_mutex_t progress_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 /* Store information on directories with xattr's. */
 struct dir_xattr *dir_xattr_list;
@@ -658,6 +659,7 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 	}
 
 	if (flags->progress) {
+		__pthread_mutex_lock(&progress_mutex);
 		fc_count++;
 		if (fc_count % STAR_COUNT == 0) {
 			if (flags->mass_relabel && efile_count > 0) {
@@ -669,6 +671,7 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 			}
 			fflush(stdout);
 		}
+		__pthread_mutex_unlock(&progress_mutex);
 	}
 
 	if (flags->add_assoc) {
-- 
2.30.2

[RFC PATCH userspace 5/6] selinux_restorecon: introduce selinux_restorecon_parallel(3)

[Ondrej Mosnacek]

Refactor selinux_restorecon(3) to allow for distributing the relabeling
to multiple threads and add a new function
selinux_restorecon_parallel(3), which allows specifying the number of
threads to use. The existing selinux_restorecon(3) function maintains
the same interface and maintains the same behavior (i.e. relabeling is
done on a single thread).

The parallel implementation takes a simple approach of performing all
the directory tree traversal in a critical section and only letting the
relabeling of individual objects run in parallel. Thankfully, this
approach turns out to be efficient enough in practice, as shown by
restorecon benchmarks (detailed in a subsequent patch that switches
setfiles & restorecon to use selinux_restorecon_parallel(3)).

Note that to be able to use the parallelism, the calling application/
library must be explicitly linked to the libpthread library (statically
or dynamically). This is necessary to mantain the requirement that
libselinux shouldn't explicitly link with libpthread. (I don't know what
exactly was the reason behind this requirement as the commit logs are
fuzzy, but special care has been taken in the past to maintain it, so I
didn't want to break it...)

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 libselinux/include/selinux/restorecon.h       |  14 +
 libselinux/man/man3/selinux_restorecon.3      |  29 ++
 .../man/man3/selinux_restorecon_parallel.3    |   1 +
 libselinux/src/libselinux.map                 |   5 +
 libselinux/src/selinux_internal.h             |  14 +
 libselinux/src/selinux_restorecon.c           | 461 ++++++++++++------
 6 files changed, 365 insertions(+), 159 deletions(-)
 create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3

diff --git a/libselinux/include/selinux/restorecon.h b/libselinux/include/selinux/restorecon.h
index 466de39a..1821a3dc 100644
--- a/libselinux/include/selinux/restorecon.h
+++ b/libselinux/include/selinux/restorecon.h
@@ -2,6 +2,7 @@
 #define _RESTORECON_H_
 
 #include <sys/types.h>
+#include <stddef.h>
 #include <stdarg.h>
 
 #ifdef __cplusplus
@@ -23,6 +24,19 @@ extern "C" {
  */
 extern int selinux_restorecon(const char *pathname,
 				    unsigned int restorecon_flags);
+/**
+ * selinux_restorecon_parallel - Relabel files, optionally use more threads.
+ * @pathname: specifies file/directory to relabel.
+ * @restorecon_flags: specifies the actions to be performed when relabeling.
+ * @nthreads: specifies the number of threads to use (0 = use number of CPUs
+ *            currently online)
+ *
+ * Same as selinux_restorecon(3), but allows to use multiple threads to do
+ * the work.
+ */
+extern int selinux_restorecon_parallel(const char *pathname,
+				       unsigned int restorecon_flags,
+				       size_t nthreads);
 /*
  * restorecon_flags options
  */
diff --git a/libselinux/man/man3/selinux_restorecon.3 b/libselinux/man/man3/selinux_restorecon.3
index ad637406..334d2930 100644
--- a/libselinux/man/man3/selinux_restorecon.3
+++ b/libselinux/man/man3/selinux_restorecon.3
@@ -11,6 +11,14 @@ selinux_restorecon \- restore file(s) default SELinux security contexts
 .br
 .BI "unsigned int " restorecon_flags ");"
 .in
+.sp
+.BI "int selinux_restorecon_parallel(const char *" pathname ,
+.in +\w'int selinux_restorecon_parallel('u
+.br
+.BI "unsigned int " restorecon_flags ","
+.br
+.BI "size_t " nthreads ");"
+.in
 .
 .SH "DESCRIPTION"
 .BR selinux_restorecon ()
@@ -187,6 +195,27 @@ unless the
 .B SELINUX_RESTORECON_IGNORE_MOUNTS
 flag has been set.
 .RE
+.sp
+.BR selinux_restorecon_parallel()
+is similar to
+.BR selinux_restorecon (3),
+but accepts another parameter that allows to run relabeling over multiple
+threads:
+.sp
+.RS
+.IR nthreads
+specifies the number of threads to use during relabeling. When set to 1,
+the behavior is the same as calling
+.BR selinux_restorecon (3).
+When set to 0, the function will try to use as many threads as there are
+online CPU cores. When set to any other number, the function will try to use
+the given number of threads.
+.sp
+Note that to use the parallel relabeling capability, the calling process
+must be linked with the
+.B libpthread
+library (either at compile time or dynamically at run time). Otherwise the
+function will print a warning and fall back to the single threaded mode.
 .
 .SH "RETURN VALUE"
 On success, zero is returned.  On error, \-1 is returned and
diff --git a/libselinux/man/man3/selinux_restorecon_parallel.3 b/libselinux/man/man3/selinux_restorecon_parallel.3
new file mode 100644
index 00000000..092d8412
--- /dev/null
+++ b/libselinux/man/man3/selinux_restorecon_parallel.3
@@ -0,0 +1 @@
+.so man3/selinux_restorecon.3
diff --git a/libselinux/src/libselinux.map b/libselinux/src/libselinux.map
index 2a368e93..68a1ad32 100644
--- a/libselinux/src/libselinux.map
+++ b/libselinux/src/libselinux.map
@@ -240,3 +240,8 @@ LIBSELINUX_1.0 {
   local:
     *;
 };
+
+LIBSELINUX_3.2 {
+  global:
+    selinux_restorecon_parallel;
+} LIBSELINUX_1.0;
diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h
index 27e9ac53..bca9398a 100644
--- a/libselinux/src/selinux_internal.h
+++ b/libselinux/src/selinux_internal.h
@@ -69,6 +69,20 @@ extern int selinux_page_size ;
 			pthread_mutex_unlock(LOCK);		\
 	} while (0)
 
+#pragma weak pthread_create
+#pragma weak pthread_cond_init
+#pragma weak pthread_cond_signal
+#pragma weak pthread_cond_destroy
+#pragma weak pthread_cond_wait
+
+/* check if all functions needed to do parallel operations are available */
+#define __pthread_supported (					\
+	pthread_create &&					\
+	pthread_cond_init &&					\
+	pthread_cond_destroy &&					\
+	pthread_cond_signal &&					\
+	pthread_cond_wait					\
+)
 
 #define SELINUXDIR "/etc/selinux/"
 #define SELINUXCONFIG SELINUXDIR "config"
diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 3f3ac2e6..adbaeeae 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -621,7 +621,7 @@ out:
 }
 
 static int restorecon_sb(const char *pathname, const struct stat *sb,
-			    struct rest_flags *flags)
+			    struct rest_flags *flags, bool first)
 {
 	char *newcon = NULL;
 	char *curcon = NULL;
@@ -633,9 +633,9 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 
 	if (rootpath) {
 		if (strncmp(rootpath, lookup_path, rootpathlen) != 0) {
-			selinux_log(SELINUX_ERROR,
-				    "%s is not located in alt_rootpath %s\n",
-				    lookup_path, rootpath);
+			selinux_log_sync(SELINUX_ERROR,
+					 "%s is not located in alt_rootpath %s\n",
+					 lookup_path, rootpath);
 			return -1;
 		}
 		lookup_path += rootpathlen;
@@ -650,10 +650,10 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 						    sb->st_mode);
 
 	if (rc < 0) {
-		if (errno == ENOENT && flags->warnonnomatch)
-			selinux_log(SELINUX_INFO,
-				    "Warning no default label for %s\n",
-				    lookup_path);
+		if (errno == ENOENT && flags->warnonnomatch && first)
+			selinux_log_sync(SELINUX_INFO,
+					 "Warning no default label for %s\n",
+					 lookup_path);
 
 		return 0; /* no match, but not an error */
 	}
@@ -678,8 +678,8 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 		rc = filespec_add(sb->st_ino, newcon, pathname, flags);
 
 		if (rc < 0) {
-			selinux_log(SELINUX_ERROR,
-				    "filespec_add error: %s\n", pathname);
+			selinux_log_sync(SELINUX_ERROR,
+					 "filespec_add error: %s\n", pathname);
 			freecon(newcon);
 			return -1;
 		}
@@ -692,8 +692,8 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 	}
 
 	if (flags->log_matches)
-		selinux_log(SELINUX_INFO, "%s matched by %s\n",
-			    pathname, newcon);
+		selinux_log_sync(SELINUX_INFO, "%s matched by %s\n",
+				 pathname, newcon);
 
 	if (lgetfilecon_raw(pathname, &curcon) < 0) {
 		if (errno != ENODATA)
@@ -706,9 +706,9 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 		if (!flags->set_specctx && curcon &&
 				    (is_context_customizable(curcon) > 0)) {
 			if (flags->verbose) {
-				selinux_log(SELINUX_INFO,
+				selinux_log_sync(SELINUX_INFO,
 				 "%s not reset as customized by admin to %s\n",
-							    pathname, curcon);
+						 pathname, curcon);
 			}
 			goto out;
 		}
@@ -734,10 +734,10 @@ static int restorecon_sb(const char *pathname, const struct stat *sb,
 		}
 
 		if (flags->verbose)
-			selinux_log(SELINUX_INFO,
-				    "%s %s from %s to %s\n",
-				    updated ? "Relabeled" : "Would relabel",
-				    pathname, curcon, newcon);
+			selinux_log_sync(SELINUX_INFO,
+					 "%s %s from %s to %s\n",
+					 updated ? "Relabeled" : "Would relabel",
+					 pathname, curcon, newcon);
 
 		if (flags->syslog_changes && !flags->nochange) {
 			if (curcon)
@@ -757,9 +757,9 @@ out1:
 	freecon(newcon);
 	return rc;
 err:
-	selinux_log(SELINUX_ERROR,
-		    "Could not set context for %s:  %s\n",
-		    pathname, strerror(errno));
+	selinux_log_sync(SELINUX_ERROR,
+			 "Could not set context for %s:  %s\n",
+			 pathname, strerror(errno));
 	rc = -1;
 	goto out1;
 }
@@ -825,65 +825,217 @@ oom:
 	goto free;
 }
 
+struct rest_state {
+	struct rest_flags flags;
+	dev_t dev_num;
+	struct statfs sfsb;
+	bool ignore_digest;
+	bool setrestorecondigest;
+	bool parallel;
 
-/*
- * Public API
- */
+	FTS *fts;
+	FTSENT *ftsent_first;
+	struct dir_hash_node *head, *current;
+	bool abort;
+	int error;
+	int saved_errno;
+	size_t inprogress;
+	pthread_cond_t cond_finish;
+	pthread_mutex_t mutex;
+};
 
-/* selinux_restorecon(3) - Main function that is responsible for labeling */
-int selinux_restorecon(const char *pathname_orig,
-		       unsigned int restorecon_flags)
+static void *selinux_restorecon_thread(void *arg)
 {
-	struct rest_flags flags;
+	struct rest_state *state = arg;
+	FTS *fts = state->fts;
+	FTSENT *ftsent;
+	int error;
+	char ent_path[PATH_MAX];
+	struct stat ent_st;
+	bool first = false;
+
+	if (state->parallel)
+		pthread_mutex_lock(&state->mutex);
+
+	if (state->ftsent_first) {
+		ftsent = state->ftsent_first;
+		state->ftsent_first = NULL;
+		first = true;
+		goto loop_body;
+	}
+
+	while ((errno = 0, ftsent = fts_read(fts)) != NULL) {
+loop_body:
+		/* If the FTS_XDEV flag is set and the device is different */
+		if (state->flags.set_xdev &&
+		    ftsent->fts_statp->st_dev != state->dev_num)
+			continue;
+
+		switch (ftsent->fts_info) {
+		case FTS_DC:
+			selinux_log_sync(SELINUX_ERROR,
+					 "Directory cycle on %s.\n",
+					 ftsent->fts_path);
+			errno = ELOOP;
+			state->error = -1;
+			state->abort = true;
+			goto finish;
+		case FTS_DP:
+			continue;
+		case FTS_DNR:
+			selinux_log_sync(SELINUX_ERROR,
+					 "Could not read %s: %s.\n",
+					 ftsent->fts_path,
+					 strerror(ftsent->fts_errno));
+			fts_set(fts, ftsent, FTS_SKIP);
+			continue;
+		case FTS_NS:
+			selinux_log_sync(SELINUX_ERROR,
+					 "Could not stat %s: %s.\n",
+					 ftsent->fts_path,
+					 strerror(ftsent->fts_errno));
+			fts_set(fts, ftsent, FTS_SKIP);
+			continue;
+		case FTS_ERR:
+			selinux_log_sync(SELINUX_ERROR,
+					 "Error on %s: %s.\n",
+					 ftsent->fts_path,
+					 strerror(ftsent->fts_errno));
+			fts_set(fts, ftsent, FTS_SKIP);
+			continue;
+		case FTS_D:
+			if (state->sfsb.f_type == SYSFS_MAGIC &&
+			    !selabel_partial_match(fc_sehandle,
+			    ftsent->fts_path)) {
+				fts_set(fts, ftsent, FTS_SKIP);
+				continue;
+			}
+
+			if (check_excluded(ftsent->fts_path)) {
+				fts_set(fts, ftsent, FTS_SKIP);
+				continue;
+			}
+
+			if (state->setrestorecondigest) {
+				struct dir_hash_node *new_node = NULL;
+
+				if (check_context_match_for_dir(ftsent->fts_path,
+								&new_node,
+								state->error) &&
+								!state->ignore_digest) {
+					selinux_log_sync(SELINUX_INFO,
+							 "Skipping restorecon on directory(%s)\n",
+							 ftsent->fts_path);
+					fts_set(fts, ftsent, FTS_SKIP);
+					continue;
+				}
+
+				if (new_node && !state->error) {
+					if (!state->current) {
+						state->current = new_node;
+						state->head = state->current;
+					} else {
+						state->current->next = new_node;
+						state->current = new_node;
+					}
+				}
+			}
+			/* fall through */
+		default:
+			strcpy(ent_path, ftsent->fts_path);
+			ent_st = *ftsent->fts_statp;
+			if (state->parallel) {
+				state->inprogress += 1;
+				pthread_mutex_unlock(&state->mutex);
+			}
+
+			error = restorecon_sb(ent_path, &ent_st, &state->flags,
+					      first);
+
+			if (state->parallel) {
+				pthread_mutex_lock(&state->mutex);
+				state->inprogress -= 1;
+				if (state->abort)
+					goto unlock;
+			}
+
+			state->error |= error;
+			first = false;
+			if (error && state->flags.abort_on_error) {
+				state->abort = true;
+				goto finish;
+			}
+			break;
+		}
+	}
+
+finish:
+	if (!state->saved_errno)
+		state->saved_errno = errno;
+unlock:
+	if (state->parallel) {
+		if (state->inprogress == 0)
+			pthread_cond_signal(&state->cond_finish);
+		pthread_mutex_unlock(&state->mutex);
+	}
+	return NULL;
+}
+
+static int selinux_restorecon_common(const char *pathname_orig,
+				     unsigned int restorecon_flags,
+				     size_t nthreads)
+{
+	struct rest_state state;
 
-	flags.nochange = (restorecon_flags &
+	state.flags.nochange = (restorecon_flags &
 		    SELINUX_RESTORECON_NOCHANGE) ? true : false;
-	flags.verbose = (restorecon_flags &
+	state.flags.verbose = (restorecon_flags &
 		    SELINUX_RESTORECON_VERBOSE) ? true : false;
-	flags.progress = (restorecon_flags &
+	state.flags.progress = (restorecon_flags &
 		    SELINUX_RESTORECON_PROGRESS) ? true : false;
-	flags.mass_relabel = (restorecon_flags &
+	state.flags.mass_relabel = (restorecon_flags &
 		    SELINUX_RESTORECON_MASS_RELABEL) ? true : false;
-	flags.recurse = (restorecon_flags &
+	state.flags.recurse = (restorecon_flags &
 		    SELINUX_RESTORECON_RECURSE) ? true : false;
-	flags.set_specctx = (restorecon_flags &
+	state.flags.set_specctx = (restorecon_flags &
 		    SELINUX_RESTORECON_SET_SPECFILE_CTX) ? true : false;
-	flags.userealpath = (restorecon_flags &
+	state.flags.userealpath = (restorecon_flags &
 		   SELINUX_RESTORECON_REALPATH) ? true : false;
-	flags.set_xdev = (restorecon_flags &
+	state.flags.set_xdev = (restorecon_flags &
 		   SELINUX_RESTORECON_XDEV) ? true : false;
-	flags.add_assoc = (restorecon_flags &
+	state.flags.add_assoc = (restorecon_flags &
 		   SELINUX_RESTORECON_ADD_ASSOC) ? true : false;
-	flags.abort_on_error = (restorecon_flags &
+	state.flags.abort_on_error = (restorecon_flags &
 		   SELINUX_RESTORECON_ABORT_ON_ERROR) ? true : false;
-	flags.syslog_changes = (restorecon_flags &
+	state.flags.syslog_changes = (restorecon_flags &
 		   SELINUX_RESTORECON_SYSLOG_CHANGES) ? true : false;
-	flags.log_matches = (restorecon_flags &
+	state.flags.log_matches = (restorecon_flags &
 		   SELINUX_RESTORECON_LOG_MATCHES) ? true : false;
-	flags.ignore_noent = (restorecon_flags &
+	state.flags.ignore_noent = (restorecon_flags &
 		   SELINUX_RESTORECON_IGNORE_NOENTRY) ? true : false;
-	flags.warnonnomatch = true;
-	flags.conflicterror = (restorecon_flags &
+	state.flags.warnonnomatch = true;
+	state.flags.conflicterror = (restorecon_flags &
 		   SELINUX_RESTORECON_CONFLICT_ERROR) ? true : false;
 	ignore_mounts = (restorecon_flags &
 		   SELINUX_RESTORECON_IGNORE_MOUNTS) ? true : false;
-	bool ignore_digest = (restorecon_flags &
+	state.ignore_digest = (restorecon_flags &
 		    SELINUX_RESTORECON_IGNORE_DIGEST) ? true : false;
-	bool setrestorecondigest = true;
+	state.setrestorecondigest = true;
+
+	state.head = NULL;
+	state.current = NULL;
+	state.abort = false;
+	state.error = 0;
+	state.saved_errno = 0;
 
 	struct stat sb;
-	struct statfs sfsb;
-	FTS *fts;
-	FTSENT *ftsent;
 	char *pathname = NULL, *pathdnamer = NULL, *pathdname, *pathbname;
 	char *paths[2] = { NULL, NULL };
 	int fts_flags, error, sverrno;
-	dev_t dev_num = 0;
 	struct dir_hash_node *current = NULL;
-	struct dir_hash_node *head = NULL;
 
-	if (flags.verbose && flags.progress)
-		flags.verbose = false;
+	if (state.flags.verbose && state.flags.progress)
+		state.flags.verbose = false;
 
 	__selinux_once(fc_once, restorecon_init);
 
@@ -896,13 +1048,31 @@ int selinux_restorecon(const char *pathname_orig,
 	 */
 	if (selabel_no_digest ||
 	    (restorecon_flags & SELINUX_RESTORECON_SKIP_DIGEST))
-		setrestorecondigest = false;
+		state.setrestorecondigest = false;
+
+	if (!__pthread_supported) {
+		if (nthreads != 1) {
+			nthreads = 1;
+			selinux_log(SELINUX_WARNING,
+				"Threading functionality not available, falling back to 1 thread.");
+		}
+	} else if (nthreads == 0) {
+		long nproc = sysconf(_SC_NPROCESSORS_ONLN);
+
+		if (nproc > 0) {
+			nthreads = nproc;
+		} else {
+			nthreads = 1;
+			selinux_log(SELINUX_WARNING,
+				"Unable to detect CPU count, falling back to 1 thread.");
+		}
+	}
 
 	/*
 	 * Convert passed-in pathname to canonical pathname by resolving
 	 * realpath of containing dir, then appending last component name.
 	 */
-	if (flags.userealpath) {
+	if (state.flags.userealpath) {
 		char *basename_cpy = strdup(pathname_orig);
 		if (!basename_cpy)
 			goto realpatherr;
@@ -947,7 +1117,7 @@ int selinux_restorecon(const char *pathname_orig,
 	paths[0] = pathname;
 
 	if (lstat(pathname, &sb) < 0) {
-		if (flags.ignore_noent && errno == ENOENT) {
+		if (state.flags.ignore_noent && errno == ENOENT) {
 			free(pathdnamer);
 			free(pathname);
 			return 0;
@@ -962,21 +1132,21 @@ int selinux_restorecon(const char *pathname_orig,
 
 	/* Skip digest if not a directory */
 	if (!S_ISDIR(sb.st_mode))
-		setrestorecondigest = false;
+		state.setrestorecondigest = false;
 
-	if (!flags.recurse) {
+	if (!state.flags.recurse) {
 		if (check_excluded(pathname)) {
 			error = 0;
 			goto cleanup;
 		}
 
-		error = restorecon_sb(pathname, &sb, &flags);
+		error = restorecon_sb(pathname, &sb, &state.flags, true);
 		goto cleanup;
 	}
 
 	/* Obtain fs type */
-	memset(&sfsb, 0, sizeof sfsb);
-	if (!S_ISLNK(sb.st_mode) && statfs(pathname, &sfsb) < 0) {
+	memset(&state.sfsb, 0, sizeof(state.sfsb));
+	if (!S_ISLNK(sb.st_mode) && statfs(pathname, &state.sfsb) < 0) {
 		selinux_log(SELINUX_ERROR,
 			    "statfs(%s) failed: %s\n",
 			    pathname, strerror(errno));
@@ -985,21 +1155,21 @@ int selinux_restorecon(const char *pathname_orig,
 	}
 
 	/* Skip digest on in-memory filesystems and /sys */
-	if (sfsb.f_type == RAMFS_MAGIC || sfsb.f_type == TMPFS_MAGIC ||
-	    sfsb.f_type == SYSFS_MAGIC)
-		setrestorecondigest = false;
+	if (state.sfsb.f_type == RAMFS_MAGIC || state.sfsb.f_type == TMPFS_MAGIC ||
+	    state.sfsb.f_type == SYSFS_MAGIC)
+		state.setrestorecondigest = false;
 
-	if (flags.set_xdev)
+	if (state.flags.set_xdev)
 		fts_flags = FTS_PHYSICAL | FTS_NOCHDIR | FTS_XDEV;
 	else
 		fts_flags = FTS_PHYSICAL | FTS_NOCHDIR;
 
-	fts = fts_open(paths, fts_flags, NULL);
-	if (!fts)
+	state.fts = fts_open(paths, fts_flags, NULL);
+	if (!state.fts)
 		goto fts_err;
 
-	ftsent = fts_read(fts);
-	if (!ftsent)
+	state.ftsent_first = fts_read(state.fts);
+	if (!state.ftsent_first)
 		goto fts_err;
 
 	/*
@@ -1011,100 +1181,54 @@ int selinux_restorecon(const char *pathname_orig,
 	 * directories with a different device number when the FTS_XDEV flag
 	 * is set (from http://marc.info/?l=selinux&m=124688830500777&w=2).
 	 */
-	dev_num = ftsent->fts_statp->st_dev;
+	state.dev_num = state.ftsent_first->fts_statp->st_dev;
 
-	error = 0;
-	do {
-		/* If the FTS_XDEV flag is set and the device is different */
-		if (flags.set_xdev && ftsent->fts_statp->st_dev != dev_num)
-			continue;
-
-		switch (ftsent->fts_info) {
-		case FTS_DC:
-			selinux_log(SELINUX_ERROR,
-				    "Directory cycle on %s.\n",
-				    ftsent->fts_path);
-			errno = ELOOP;
-			error = -1;
-			goto out;
-		case FTS_DP:
-			continue;
-		case FTS_DNR:
-			selinux_log(SELINUX_ERROR,
-				    "Could not read %s: %s.\n",
-				    ftsent->fts_path,
-						  strerror(ftsent->fts_errno));
-			fts_set(fts, ftsent, FTS_SKIP);
-			continue;
-		case FTS_NS:
-			selinux_log(SELINUX_ERROR,
-				    "Could not stat %s: %s.\n",
-				    ftsent->fts_path,
-						  strerror(ftsent->fts_errno));
-			fts_set(fts, ftsent, FTS_SKIP);
-			continue;
-		case FTS_ERR:
-			selinux_log(SELINUX_ERROR,
-				    "Error on %s: %s.\n",
-				    ftsent->fts_path,
-						  strerror(ftsent->fts_errno));
-			fts_set(fts, ftsent, FTS_SKIP);
-			continue;
-		case FTS_D:
-			if (sfsb.f_type == SYSFS_MAGIC &&
-			    !selabel_partial_match(fc_sehandle,
-			    ftsent->fts_path)) {
-				fts_set(fts, ftsent, FTS_SKIP);
-				continue;
-			}
+	if (nthreads == 1) {
+		state.parallel = false;
+		selinux_restorecon_thread(&state);
+	} else {
+		size_t i;
+		pthread_t thread;
+
+		pthread_mutex_init(&state.mutex, NULL);
+		pthread_cond_init(&state.cond_finish, NULL);
+
+		state.parallel = true;
+		/*
+		 * Start (nthreads - 1) threads - this thread is going to
+		 * take part, too.
+		 */
+		for (i = 1; i < nthreads; i++) {
+			pthread_create(&thread, NULL, selinux_restorecon_thread,
+				       &state);
+			/*
+			 * If thread creation failed - doesn't matter, we just
+			 * let the successfully created threads do the job.
+			 */
+		}
 
-			if (check_excluded(ftsent->fts_path)) {
-				fts_set(fts, ftsent, FTS_SKIP);
-				continue;
-			}
+		/* Let's join in on the fun! */
+		selinux_restorecon_thread(&state);
 
-			if (setrestorecondigest) {
-				struct dir_hash_node *new_node = NULL;
+		pthread_mutex_lock(&state.mutex);
+		if (state.inprogress != 0)
+			pthread_cond_wait(&state.cond_finish, &state.mutex);
+		pthread_mutex_unlock(&state.mutex);
 
-				if (check_context_match_for_dir(ftsent->fts_path,
-								&new_node,
-								error) &&
-								!ignore_digest) {
-					selinux_log(SELINUX_INFO,
-						    "Skipping restorecon on directory(%s)\n",
-						    ftsent->fts_path);
-					fts_set(fts, ftsent, FTS_SKIP);
-					continue;
-				}
+		pthread_mutex_destroy(&state.mutex);
+		pthread_cond_destroy(&state.cond_finish);
+	}
 
-				if (new_node && !error) {
-					if (!current) {
-						current = new_node;
-						head = current;
-					} else {
-						current->next = new_node;
-						current = current->next;
-					}
-				}
-			}
-			/* fall through */
-		default:
-			error |= restorecon_sb(ftsent->fts_path,
-					       ftsent->fts_statp, &flags);
-			if (flags.warnonnomatch)
-				flags.warnonnomatch = false;
-			if (error && flags.abort_on_error)
-				goto out;
-			break;
-		}
-	} while ((ftsent = fts_read(fts)) != NULL);
+	error = state.error;
+	if (state.saved_errno)
+		goto out;
 
 	/*
 	 * Labeling successful. Write partial match digests for subdirectories.
 	 * TODO: Write digest upon FTS_DP if no error occurs in its descents.
 	 */
-	if (setrestorecondigest && !flags.nochange && !error) {
-		current = head;
+	if (state.setrestorecondigest && !state.flags.nochange && !error) {
+		current = state.head;
 		while (current != NULL) {
 			if (setxattr(current->path,
 			    RESTORECON_PARTIAL_MATCH_DIGEST,
@@ -1120,22 +1244,21 @@ int selinux_restorecon(const char *pathname_orig,
 	}
 
 out:
-	if (flags.progress && flags.mass_relabel)
+	if (state.flags.progress && state.flags.mass_relabel)
 		fprintf(stdout, "\r%s 100.0%%\n", pathname);
 
-	sverrno = errno;
-	(void) fts_close(fts);
-	errno = sverrno;
+	(void) fts_close(state.fts);
+	errno = state.saved_errno;
 cleanup:
-	if (flags.add_assoc) {
-		if (flags.verbose)
+	if (state.flags.add_assoc) {
+		if (state.flags.verbose)
 			filespec_eval();
 		filespec_destroy();
 	}
 	free(pathdnamer);
 	free(pathname);
 
-	current = head;
+	current = state.head;
 	while (current != NULL) {
 		struct dir_hash_node *next = current->next;
 
@@ -1169,6 +1292,26 @@ fts_err:
 	goto cleanup;
 }
 
+
+/*
+ * Public API
+ */
+
+/* selinux_restorecon(3) - Main function that is responsible for labeling */
+int selinux_restorecon(const char *pathname_orig,
+		       unsigned int restorecon_flags)
+{
+	return selinux_restorecon_common(pathname_orig, restorecon_flags, 1);
+}
+
+/* selinux_restorecon_parallel(3) - Parallel version of selinux_restorecon(3) */
+int selinux_restorecon_parallel(const char *pathname_orig,
+				unsigned int restorecon_flags,
+				size_t nthreads)
+{
+	return selinux_restorecon_common(pathname_orig, restorecon_flags, nthreads);
+}
+
 /* selinux_restorecon_set_sehandle(3) is called to set the global fc handle */
 void selinux_restorecon_set_sehandle(struct selabel_handle *hndl)
 {
-- 
2.30.2

[RFC PATCH userspace 6/6] setfiles/restorecon: support parallel relabeling

[Ondrej Mosnacek]

Use the newly introduced selinux_restorecon_parallel(3) in
setfiles/restorecon and a -T option to both to allow enabling parallel
relabeling. The default behavior without specifying the -T option is to
use 1 thread; parallel relabeling must be requested explicitly by
passing -T 0 (which will use as many threads as there are available CPU
cores) or -T <N>, which will use <N> threads.

=== Benchmarks ===
As measured on a 32-core cloud VM with Fedora 34. Not a fully
representative environment, but still the scaling is quite good.

WITHOUT PATCHES:
$ time restorecon -rn /usr/src

real    0m23,580s
user    0m22,681s
sys     0m0,707s

WITH PATCHES:
$ time restorecon -rn -T 1 /usr/src

real    0m23,493s
user    0m22,637s
sys     0m0,668s
$ time restorecon -rn -T 2 /usr/src

real    0m13,438s
user    0m25,536s
sys     0m0,946s
$ time restorecon -rn -T 8 /usr/src

real    0m5,476s
user    0m38,440s
sys     0m3,016s
$ time restorecon -rn -T 16 /usr/src

real    0m3,868s
user    0m51,292s
sys     0m5,289s
$ time restorecon -rn -T 32 /usr/src

real    0m2,817s
user    1m0,317s
sys     0m6,157s

Note that the benchmarks were performed in read-only mode (-n), so the
labels were only read and looked up in the database, not written. When
fixing labels on a heavily mislabeled system, the scaling would likely
be event better, since a larger % of work could be done in parallel.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 policycoreutils/setfiles/Makefile     |  2 +-
 policycoreutils/setfiles/restore.c    |  7 ++++---
 policycoreutils/setfiles/restore.h    |  2 +-
 policycoreutils/setfiles/restorecon.8 |  9 +++++++++
 policycoreutils/setfiles/setfiles.8   |  9 +++++++++
 policycoreutils/setfiles/setfiles.c   | 28 ++++++++++++++++-----------
 6 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/policycoreutils/setfiles/Makefile b/policycoreutils/setfiles/Makefile
index a3bbbe11..82de8f2a 100644
--- a/policycoreutils/setfiles/Makefile
+++ b/policycoreutils/setfiles/Makefile
@@ -6,7 +6,7 @@ MANDIR = $(PREFIX)/share/man
 AUDITH ?= $(shell test -f /usr/include/libaudit.h && echo y)
 
 CFLAGS ?= -g -Werror -Wall -W
-override LDLIBS += -lselinux -lsepol
+override LDLIBS += -lselinux -lsepol -lpthread
 
 ifeq ($(AUDITH), y)
 	override CFLAGS += -DUSE_AUDIT
diff --git a/policycoreutils/setfiles/restore.c b/policycoreutils/setfiles/restore.c
index 9d688c60..74d48bb3 100644
--- a/policycoreutils/setfiles/restore.c
+++ b/policycoreutils/setfiles/restore.c
@@ -72,7 +72,7 @@ void restore_finish(void)
 	}
 }
 
-int process_glob(char *name, struct restore_opts *opts)
+int process_glob(char *name, struct restore_opts *opts, size_t nthreads)
 {
 	glob_t globbuf;
 	size_t i = 0;
@@ -91,8 +91,9 @@ int process_glob(char *name, struct restore_opts *opts)
 			continue;
 		if (len > 0 && strcmp(&globbuf.gl_pathv[i][len], "/..") == 0)
 			continue;
-		rc = selinux_restorecon(globbuf.gl_pathv[i],
-					opts->restorecon_flags);
+		rc = selinux_restorecon_parallel(globbuf.gl_pathv[i],
+						 opts->restorecon_flags,
+						 nthreads);
 		if (rc < 0)
 			errors = rc;
 	}
diff --git a/policycoreutils/setfiles/restore.h b/policycoreutils/setfiles/restore.h
index ac6ad680..bb35a1db 100644
--- a/policycoreutils/setfiles/restore.h
+++ b/policycoreutils/setfiles/restore.h
@@ -49,7 +49,7 @@ struct restore_opts {
 void restore_init(struct restore_opts *opts);
 void restore_finish(void);
 void add_exclude(const char *directory);
-int process_glob(char *name, struct restore_opts *opts);
+int process_glob(char *name, struct restore_opts *opts, size_t nthreads);
 extern char **exclude_list;
 
 #endif
diff --git a/policycoreutils/setfiles/restorecon.8 b/policycoreutils/setfiles/restorecon.8
index 668486f6..e07db2c8 100644
--- a/policycoreutils/setfiles/restorecon.8
+++ b/policycoreutils/setfiles/restorecon.8
@@ -33,6 +33,8 @@ restorecon \- restore file(s) default SELinux security contexts.
 .RB [ \-W ]
 .RB [ \-I | \-D ]
 .RB [ \-x ]
+.RB [ \-T
+.IR nthreads ]
 
 .SH "DESCRIPTION"
 This manual page describes the
@@ -160,6 +162,13 @@ prevent
 .B restorecon
 from crossing file system boundaries.
 .TP
+.BI \-T \ nthreads
+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
+number to use the given number of threads (if possible).
+.TP
 .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 4d28bc9a..15f939d1 100644
--- a/policycoreutils/setfiles/setfiles.8
+++ b/policycoreutils/setfiles/setfiles.8
@@ -19,6 +19,8 @@ setfiles \- set SELinux file security contexts.
 .RB [ \-W ]
 .RB [ \-F ]
 .RB [ \-I | \-D ]
+.RB [ \-T
+.IR nthreads ]
 .I spec_file
 .IR pathname \ ...
 
@@ -161,6 +163,13 @@ quote marks or backslashes.  The
 option of GNU
 .B find
 produces input suitable for this mode.
+.TP
+.BI \-T \ nthreads
+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
+number to use the given number of threads (if possible).
 
 .SH "ARGUMENTS"
 .TP
diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
index f018d161..2313a21f 100644
--- a/policycoreutils/setfiles/setfiles.c
+++ b/policycoreutils/setfiles/setfiles.c
@@ -1,4 +1,5 @@
 #include "restore.h"
+#include <stdlib.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdio_ext.h>
@@ -34,14 +35,14 @@ static __attribute__((__noreturn__)) void usage(const char *const name)
 {
 	if (iamrestorecon) {
 		fprintf(stderr,
-			"usage:  %s [-iIDFmnprRv0x] [-e excludedir] pathname...\n"
-			"usage:  %s [-iIDFmnprRv0x] [-e excludedir] -f filename\n",
+			"usage:  %s [-iIDFmnprRv0xT] [-e excludedir] pathname...\n"
+			"usage:  %s [-iIDFmnprRv0xT] [-e excludedir] -f filename\n",
 			name, name);
 	} else {
 		fprintf(stderr,
-			"usage:  %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file pathname...\n"
-			"usage:  %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file -f filename\n"
-			"usage:  %s -s [-diIDlmnpqvFW] spec_file\n",
+			"usage:  %s [-diIDlmnpqvEFWT] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file pathname...\n"
+			"usage:  %s [-diIDlmnpqvEFWT] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file -f filename\n"
+			"usage:  %s -s [-diIDlmnpqvFWT] spec_file\n",
 			name, name, name);
 	}
 	exit(-1);
@@ -144,12 +145,12 @@ int main(int argc, char **argv)
 	int opt, i = 0;
 	const char *input_filename = NULL;
 	int use_input_file = 0;
-	char *buf = NULL;
-	size_t buf_len;
+	char *buf = NULL, *endptr;
+	size_t buf_len, nthreads = 1;
 	const char *base;
 	int errors = 0;
-	const char *ropts = "e:f:hiIDlmno:pqrsvFRW0x";
-	const char *sopts = "c:de:f:hiIDlmno:pqr:svEFR:W0";
+	const char *ropts = "e:f:hiIDlmno:pqrsvFRW0xT:";
+	const char *sopts = "c:de:f:hiIDlmno:pqr:svEFR:W0T:";
 	const char *opts;
 	union selinux_callback cb;
 
@@ -370,6 +371,11 @@ int main(int argc, char **argv)
 				usage(argv[0]);
                         }
                         break;
+		case 'T':
+			nthreads = strtoull(optarg, &endptr, 10);
+			if (*optarg == '\0' || *endptr != '\0')
+				usage(argv[0]);
+			break;
 		case 'h':
 		case '?':
 			usage(argv[0]);
@@ -448,13 +454,13 @@ int main(int argc, char **argv)
 			buf[len - 1] = 0;
 			if (!strcmp(buf, "/"))
 				r_opts.mass_relabel = SELINUX_RESTORECON_MASS_RELABEL;
-			errors |= process_glob(buf, &r_opts) < 0;
+			errors |= process_glob(buf, &r_opts, nthreads) < 0;
 		}
 		if (strcmp(input_filename, "-") != 0)
 			fclose(f);
 	} else {
 		for (i = optind; i < argc; i++)
-			errors |= process_glob(argv[i], &r_opts) < 0;
+			errors |= process_glob(argv[i], &r_opts, nthreads) < 0;
 	}
 
 	maybe_audit_mass_relabel(r_opts.mass_relabel, errors);
-- 
2.30.2

Re: [RFC PATCH userspace 0/6] Parallel setfiles/restorecon

[peter enderborg]

On 3/23/21 6:08 PM, Ondrej Mosnacek wrote:
> This series adds basic support for parallel relabeling to the libselinux
> API and the setfiles/restorecon CLI tools. It turns out that doing the
> relabeling in parallel can significantly reduce the time even with a
> relatively simple approach.
Nice! Have you any figures? Is it valid for both solid state and mechanical storage?
> The first patch is a small cleanup that was found along the way and can
> be applied independently. Patches 2-4 are small incremental changes that
> make the internal selinux_restorecon functions more thread-safe (I kept
> them separate for ease of of review, but maybe they should be rather
> folded into the netx patch...). Patch 5 then completes the parallel
> relabeling implementation at libselinux level and adds a new function
> to the API that allows to make use of it. Finally, patch 6 adds parallel
> relabeling support to he setfiles/restorecon tools.
>
> The relevant man pages are also updated to reflect the new
> functionality.
>
> The patch descriptions contain more details, namely the last patch has
> also some benchmark numbers.
>
> Please test and review. I'm still not fully confident I got everything
> right (esp. regarding error handling), but I wanted to put this forward
> as an RFC to get some early feedback.
>
> Ondrej Mosnacek (6):
>   selinux_restorecon: simplify fl_head allocation by using calloc()
>   selinux_restorecon: protect file_spec list with a mutex
>   selinux_restorecon: introduce selinux_log_sync()
>   selinux_restorecon: add a global mutex to synchronize progress output
>   selinux_restorecon: introduce selinux_restorecon_parallel(3)
>   setfiles/restorecon: support parallel relabeling
>
>  libselinux/include/selinux/restorecon.h       |  14 +
>  libselinux/man/man3/selinux_restorecon.3      |  29 +
>  .../man/man3/selinux_restorecon_parallel.3    |   1 +
>  libselinux/src/libselinux.map                 |   5 +
>  libselinux/src/selinux_internal.h             |  14 +
>  libselinux/src/selinux_restorecon.c           | 498 ++++++++++++------
>  policycoreutils/setfiles/Makefile             |   2 +-
>  policycoreutils/setfiles/restore.c            |   7 +-
>  policycoreutils/setfiles/restore.h            |   2 +-
>  policycoreutils/setfiles/restorecon.8         |   9 +
>  policycoreutils/setfiles/setfiles.8           |   9 +
>  policycoreutils/setfiles/setfiles.c           |  28 +-
>  12 files changed, 436 insertions(+), 182 deletions(-)
>  create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3
>

Re: [RFC PATCH userspace 0/6] Parallel setfiles/restorecon

[Ondrej Mosnacek]

On Wed, Mar 24, 2021 at 10:59 AM peter enderborg
<peter.enderborg@sony.com> wrote:
> On 3/23/21 6:08 PM, Ondrej Mosnacek wrote:
> > This series adds basic support for parallel relabeling to the libselinux
> > API and the setfiles/restorecon CLI tools. It turns out that doing the
> > relabeling in parallel can significantly reduce the time even with a
> > relatively simple approach.
> Nice! Have you any figures? Is it valid for both solid state and mechanical storage?

They are in the last patch :) The VM setup I measured that on probably
had the storage backed up by an SSD (or something with similar
characteristics). I haven't tried it on an HDD yet.

> > The first patch is a small cleanup that was found along the way and can
> > be applied independently. Patches 2-4 are small incremental changes that
> > make the internal selinux_restorecon functions more thread-safe (I kept
> > them separate for ease of of review, but maybe they should be rather
> > folded into the netx patch...). Patch 5 then completes the parallel
> > relabeling implementation at libselinux level and adds a new function
> > to the API that allows to make use of it. Finally, patch 6 adds parallel
> > relabeling support to he setfiles/restorecon tools.
> >
> > The relevant man pages are also updated to reflect the new
> > functionality.
> >
> > The patch descriptions contain more details, namely the last patch has
> > also some benchmark numbers.
> >
> > Please test and review. I'm still not fully confident I got everything
> > right (esp. regarding error handling), but I wanted to put this forward
> > as an RFC to get some early feedback.
> >
> > Ondrej Mosnacek (6):
> >   selinux_restorecon: simplify fl_head allocation by using calloc()
> >   selinux_restorecon: protect file_spec list with a mutex
> >   selinux_restorecon: introduce selinux_log_sync()
> >   selinux_restorecon: add a global mutex to synchronize progress output
> >   selinux_restorecon: introduce selinux_restorecon_parallel(3)
> >   setfiles/restorecon: support parallel relabeling
> >
> >  libselinux/include/selinux/restorecon.h       |  14 +
> >  libselinux/man/man3/selinux_restorecon.3      |  29 +
> >  .../man/man3/selinux_restorecon_parallel.3    |   1 +
> >  libselinux/src/libselinux.map                 |   5 +
> >  libselinux/src/selinux_internal.h             |  14 +
> >  libselinux/src/selinux_restorecon.c           | 498 ++++++++++++------
> >  policycoreutils/setfiles/Makefile             |   2 +-
> >  policycoreutils/setfiles/restore.c            |   7 +-
> >  policycoreutils/setfiles/restore.h            |   2 +-
> >  policycoreutils/setfiles/restorecon.8         |   9 +
> >  policycoreutils/setfiles/setfiles.8           |   9 +
> >  policycoreutils/setfiles/setfiles.c           |  28 +-
> >  12 files changed, 436 insertions(+), 182 deletions(-)
> >  create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3
> >
>


--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

Re: [RFC PATCH userspace 0/6] Parallel setfiles/restorecon

[Nicolas Iooss]

On Wed, Mar 24, 2021 at 12:05 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Wed, Mar 24, 2021 at 10:59 AM peter enderborg
> <peter.enderborg@sony.com> wrote:
> > On 3/23/21 6:08 PM, Ondrej Mosnacek wrote:
> > > This series adds basic support for parallel relabeling to the libselinux
> > > API and the setfiles/restorecon CLI tools. It turns out that doing the
> > > relabeling in parallel can significantly reduce the time even with a
> > > relatively simple approach.
> > Nice! Have you any figures? Is it valid for both solid state and mechanical storage?
>
> They are in the last patch :) The VM setup I measured that on probably
> had the storage backed up by an SSD (or something with similar
> characteristics). I haven't tried it on an HDD yet.
>
> > > The first patch is a small cleanup that was found along the way and can
> > > be applied independently. Patches 2-4 are small incremental changes that
> > > make the internal selinux_restorecon functions more thread-safe (I kept
> > > them separate for ease of of review, but maybe they should be rather
> > > folded into the netx patch...). Patch 5 then completes the parallel
> > > relabeling implementation at libselinux level and adds a new function
> > > to the API that allows to make use of it. Finally, patch 6 adds parallel
> > > relabeling support to he setfiles/restorecon tools.
> > >
> > > The relevant man pages are also updated to reflect the new
> > > functionality.
> > >
> > > The patch descriptions contain more details, namely the last patch has
> > > also some benchmark numbers.
> > >
> > > Please test and review. I'm still not fully confident I got everything
> > > right (esp. regarding error handling), but I wanted to put this forward
> > > as an RFC to get some early feedback.
> > >
> > > Ondrej Mosnacek (6):
> > >   selinux_restorecon: simplify fl_head allocation by using calloc()
> > >   selinux_restorecon: protect file_spec list with a mutex
> > >   selinux_restorecon: introduce selinux_log_sync()
> > >   selinux_restorecon: add a global mutex to synchronize progress output
> > >   selinux_restorecon: introduce selinux_restorecon_parallel(3)
> > >   setfiles/restorecon: support parallel relabeling
> > >
> > >  libselinux/include/selinux/restorecon.h       |  14 +
> > >  libselinux/man/man3/selinux_restorecon.3      |  29 +
> > >  .../man/man3/selinux_restorecon_parallel.3    |   1 +
> > >  libselinux/src/libselinux.map                 |   5 +
> > >  libselinux/src/selinux_internal.h             |  14 +
> > >  libselinux/src/selinux_restorecon.c           | 498 ++++++++++++------
> > >  policycoreutils/setfiles/Makefile             |   2 +-
> > >  policycoreutils/setfiles/restore.c            |   7 +-
> > >  policycoreutils/setfiles/restore.h            |   2 +-
> > >  policycoreutils/setfiles/restorecon.8         |   9 +
> > >  policycoreutils/setfiles/setfiles.8           |   9 +
> > >  policycoreutils/setfiles/setfiles.c           |  28 +-
> > >  12 files changed, 436 insertions(+), 182 deletions(-)
> > >  create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3
> > >
> >

Hello,
I haven't seen any review of this RFC, so I decided to take a look.
The result looks quite good :) What is the current status of this
series, and can it become a "to-be-merged" patch series?

Anyway, here are some comments.

First, I was a little puzzled by the introduction of
selinux_log_sync() and the fact that it is used by
selinux_restorecon_common(), which means that callers of
selinux_restorecon() will also take the mutex log_mutex. This
surprised me because the description of one commit was very clear
about not depending very hard on libpthread... but in fact your code
is right there. I have just re-discovered the pthread helpers in
libselinux/src/selinux_internal.h :)

Nevertheless, now that I saw the pthread helpers, which enable using
libselinux without linking with libpthread, I am wondering: why is
introducing selinux_log_sync() like you do (and keeping calls to
selinux_log_sync() and selinux_log() in the code and praying that all
invocations from parallel code are converted to selinux_log_sync() )
better than introducing the mutex directly "in selinux_log()"? I
understand this is not so easy, because selinux_log is in fact a
function pointer... what I have in my mind consists in renaming the
pointer and in renaming a selinux_log_sync() to selinux_log(). This
would make the code less error-prone, regarding the issue of ensuring
to never call selinux_log callback in two parallel threads. What do
you think?

Then, when I compiled your patches with clang and some warning flags,
I got this warning:

selinux_restorecon.c:867:19: error: possible misuse of comma operator
here [-Werror,-Wcomma]
        while ((errno = 0, ftsent = fts_read(fts)) != NULL) {
                         ^
selinux_restorecon.c:867:10: note: cast expression to void to silence warning
        while ((errno = 0, ftsent = fts_read(fts)) != NULL) {
                ^~~~~~~~~
                (void)(  )
/usr/include/errno.h:38:16: note: expanded from macro 'errno'
# define errno (*__errno_location ())
               ^
1 error generated.

Using a comma operator seems to be right, here, and using the
suggested workaround to silence the compiler warning seems to be fine.

Finally, the generated file
libselinux/src/selinuxswig_python_exception.i needs to be upgraded,
because the new function selinux_restorecon_parallel is being added to
it. It would be nice to have a patch which updates this file, or to
have this update in patch "selinux_restorecon: introduce
selinux_restorecon_parallel(3)" (your choice).

Thanks!
Nicolas

Re: [RFC PATCH userspace 0/6] Parallel setfiles/restorecon

[Ondrej Mosnacek]

On Wed, Apr 28, 2021 at 11:12 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
> On Wed, Mar 24, 2021 at 12:05 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > On Wed, Mar 24, 2021 at 10:59 AM peter enderborg
> > <peter.enderborg@sony.com> wrote:
> > > On 3/23/21 6:08 PM, Ondrej Mosnacek wrote:
> > > > This series adds basic support for parallel relabeling to the libselinux
> > > > API and the setfiles/restorecon CLI tools. It turns out that doing the
> > > > relabeling in parallel can significantly reduce the time even with a
> > > > relatively simple approach.
> > > Nice! Have you any figures? Is it valid for both solid state and mechanical storage?
> >
> > They are in the last patch :) The VM setup I measured that on probably
> > had the storage backed up by an SSD (or something with similar
> > characteristics). I haven't tried it on an HDD yet.
> >
> > > > The first patch is a small cleanup that was found along the way and can
> > > > be applied independently. Patches 2-4 are small incremental changes that
> > > > make the internal selinux_restorecon functions more thread-safe (I kept
> > > > them separate for ease of of review, but maybe they should be rather
> > > > folded into the netx patch...). Patch 5 then completes the parallel
> > > > relabeling implementation at libselinux level and adds a new function
> > > > to the API that allows to make use of it. Finally, patch 6 adds parallel
> > > > relabeling support to he setfiles/restorecon tools.
> > > >
> > > > The relevant man pages are also updated to reflect the new
> > > > functionality.
> > > >
> > > > The patch descriptions contain more details, namely the last patch has
> > > > also some benchmark numbers.
> > > >
> > > > Please test and review. I'm still not fully confident I got everything
> > > > right (esp. regarding error handling), but I wanted to put this forward
> > > > as an RFC to get some early feedback.
> > > >
> > > > Ondrej Mosnacek (6):
> > > >   selinux_restorecon: simplify fl_head allocation by using calloc()
> > > >   selinux_restorecon: protect file_spec list with a mutex
> > > >   selinux_restorecon: introduce selinux_log_sync()
> > > >   selinux_restorecon: add a global mutex to synchronize progress output
> > > >   selinux_restorecon: introduce selinux_restorecon_parallel(3)
> > > >   setfiles/restorecon: support parallel relabeling
> > > >
> > > >  libselinux/include/selinux/restorecon.h       |  14 +
> > > >  libselinux/man/man3/selinux_restorecon.3      |  29 +
> > > >  .../man/man3/selinux_restorecon_parallel.3    |   1 +
> > > >  libselinux/src/libselinux.map                 |   5 +
> > > >  libselinux/src/selinux_internal.h             |  14 +
> > > >  libselinux/src/selinux_restorecon.c           | 498 ++++++++++++------
> > > >  policycoreutils/setfiles/Makefile             |   2 +-
> > > >  policycoreutils/setfiles/restore.c            |   7 +-
> > > >  policycoreutils/setfiles/restore.h            |   2 +-
> > > >  policycoreutils/setfiles/restorecon.8         |   9 +
> > > >  policycoreutils/setfiles/setfiles.8           |   9 +
> > > >  policycoreutils/setfiles/setfiles.c           |  28 +-
> > > >  12 files changed, 436 insertions(+), 182 deletions(-)
> > > >  create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3
> > > >
> > >
>
> Hello,
> I haven't seen any review of this RFC, so I decided to take a look.
> The result looks quite good :) What is the current status of this
> series, and can it become a "to-be-merged" patch series?

I hope it can :) I posted it as an RFC initially mainly to get some
high-level feedback whether I'm going in the right direction. But
based on your reply it seems it's already close enough to
acceptability that I could drop the RFC in the next revision. I also
didn't test it all that thoroughly, especially w.r.t. various
corner-cases...

>
> Anyway, here are some comments.
>
> First, I was a little puzzled by the introduction of
> selinux_log_sync() and the fact that it is used by
> selinux_restorecon_common(), which means that callers of
> selinux_restorecon() will also take the mutex log_mutex. This
> surprised me because the description of one commit was very clear
> about not depending very hard on libpthread... but in fact your code
> is right there. I have just re-discovered the pthread helpers in
> libselinux/src/selinux_internal.h :)
>
> Nevertheless, now that I saw the pthread helpers, which enable using
> libselinux without linking with libpthread, I am wondering: why is
> introducing selinux_log_sync() like you do (and keeping calls to
> selinux_log_sync() and selinux_log() in the code and praying that all
> invocations from parallel code are converted to selinux_log_sync() )
> better than introducing the mutex directly "in selinux_log()"? I
> understand this is not so easy, because selinux_log is in fact a
> function pointer... what I have in my mind consists in renaming the
> pointer and in renaming a selinux_log_sync() to selinux_log(). This
> would make the code less error-prone, regarding the issue of ensuring
> to never call selinux_log callback in two parallel threads. What do
> you think?

That's a good question. Since this will be the first function in
libselinux with some internal parallelism, I guess I just didn't want
to affect the status quo of existing code too much... Indeed the lock
would be taken also in the single-threaded implementation, but since
in selinux_restorecon() selinux_log() is called only in non-hot paths,
I didn't bother optimizing that.

Anyway, I agree that making selinux_log() synchronized globally may be
a good idea. The cost should be minimal and it would prevent
accidental race conditions. If some reasonable quorum of maintainers
agrees, I will make that change in v2.

> Then, when I compiled your patches with clang and some warning flags,
> I got this warning:
>
> selinux_restorecon.c:867:19: error: possible misuse of comma operator
> here [-Werror,-Wcomma]
>         while ((errno = 0, ftsent = fts_read(fts)) != NULL) {
>                          ^
> selinux_restorecon.c:867:10: note: cast expression to void to silence warning
>         while ((errno = 0, ftsent = fts_read(fts)) != NULL) {
>                 ^~~~~~~~~
>                 (void)(  )
> /usr/include/errno.h:38:16: note: expanded from macro 'errno'
> # define errno (*__errno_location ())
>                ^
> 1 error generated.
>
> Using a comma operator seems to be right, here, and using the
> suggested workaround to silence the compiler warning seems to be fine.

Hm, yes, I think I'll just add the cast to void there... Not a big fan
of the comma operator, but I couldn't resist the temptation to use it
here :)

> Finally, the generated file
> libselinux/src/selinuxswig_python_exception.i needs to be upgraded,
> because the new function selinux_restorecon_parallel is being added to
> it. It would be nice to have a patch which updates this file, or to
> have this update in patch "selinux_restorecon: introduce
> selinux_restorecon_parallel(3)" (your choice).

Ah, thanks for pointing that out. I'll address it in v2.

--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.