* [PATCH userspace v2 0/6] Parallel setfiles/restorecon
@ 2021-10-14 14:53 Ondrej Mosnacek
2021-10-14 14:53 ` [PATCH userspace v2 1/6] selinux_restorecon: simplify fl_head allocation by using calloc() Ondrej Mosnacek
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Ondrej Mosnacek @ 2021-10-14 14:53 UTC (permalink / raw)
To: selinux
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.
Changes v1->v2:
- make selinux_log() synchronized instead of introducing selinux_log_sync()
- fix -Wcomma warning
- update the swig files as well
- bump new symbol version to LIBSELINUX_3.3 (this may need further update
depending on when this gets merged)
Ondrej Mosnacek (6):
selinux_restorecon: simplify fl_head allocation by using calloc()
selinux_restorecon: protect file_spec list with a mutex
libselinux: make selinux_log() thread-safe
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/callbacks.c | 8 +-
libselinux/src/callbacks.h | 13 +-
libselinux/src/libselinux.map | 5 +
libselinux/src/selinux_internal.h | 14 +
libselinux/src/selinux_restorecon.c | 466 ++++++++++++------
libselinux/src/selinuxswig_python.i | 6 +-
libselinux/src/selinuxswig_python_exception.i | 8 +
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 +-
16 files changed, 444 insertions(+), 177 deletions(-)
create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3
--
2.31.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH userspace v2 1/6] selinux_restorecon: simplify fl_head allocation by using calloc() 2021-10-14 14:53 [PATCH userspace v2 0/6] Parallel setfiles/restorecon Ondrej Mosnacek @ 2021-10-14 14:53 ` Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 2/6] selinux_restorecon: protect file_spec list with a mutex Ondrej Mosnacek ` (5 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Ondrej Mosnacek @ 2021-10-14 14:53 UTC (permalink / raw) To: selinux 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 04d95650..15129336 100644 --- a/libselinux/src/selinux_restorecon.c +++ b/libselinux/src/selinux_restorecon.c @@ -425,10 +425,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.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH userspace v2 2/6] selinux_restorecon: protect file_spec list with a mutex 2021-10-14 14:53 [PATCH userspace v2 0/6] Parallel setfiles/restorecon Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 1/6] selinux_restorecon: simplify fl_head allocation by using calloc() Ondrej Mosnacek @ 2021-10-14 14:53 ` Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 3/6] libselinux: make selinux_log() thread-safe Ondrej Mosnacek ` (4 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Ondrej Mosnacek @ 2021-10-14 14:53 UTC (permalink / raw) To: selinux 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 15129336..732f0ccc 100644 --- a/libselinux/src/selinux_restorecon.c +++ b/libselinux/src/selinux_restorecon.c @@ -411,6 +411,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 @@ -424,6 +425,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) @@ -444,11 +447,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", @@ -457,6 +460,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"); @@ -481,13 +487,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.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH userspace v2 3/6] libselinux: make selinux_log() thread-safe 2021-10-14 14:53 [PATCH userspace v2 0/6] Parallel setfiles/restorecon Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 1/6] selinux_restorecon: simplify fl_head allocation by using calloc() Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 2/6] selinux_restorecon: protect file_spec list with a mutex Ondrej Mosnacek @ 2021-10-14 14:53 ` Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 4/6] selinux_restorecon: add a global mutex to synchronize progress output Ondrej Mosnacek ` (3 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Ondrej Mosnacek @ 2021-10-14 14:53 UTC (permalink / raw) To: selinux Ensure that selinux_log() is thread-safe by guarding the call to the underlying callback with a mutex. Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- libselinux/src/callbacks.c | 8 +++++--- libselinux/src/callbacks.h | 13 ++++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/libselinux/src/callbacks.c b/libselinux/src/callbacks.c index c18ccc54..469c4055 100644 --- a/libselinux/src/callbacks.c +++ b/libselinux/src/callbacks.c @@ -10,6 +10,8 @@ #include <selinux/selinux.h> #include "callbacks.h" +pthread_mutex_t log_mutex = PTHREAD_MUTEX_INITIALIZER; + /* default implementations */ static int __attribute__ ((format(printf, 2, 3))) default_selinux_log(int type __attribute__((unused)), const char *fmt, ...) @@ -56,7 +58,7 @@ default_selinux_policyload(int seqno __attribute__((unused))) /* callback pointers */ int __attribute__ ((format(printf, 2, 3))) -(*selinux_log)(int, const char *, ...) = +(*selinux_log_direct)(int, const char *, ...) = default_selinux_log; int @@ -81,7 +83,7 @@ selinux_set_callback(int type, union selinux_callback cb) { switch (type) { case SELINUX_CB_LOG: - selinux_log = cb.func_log; + selinux_log_direct = cb.func_log; break; case SELINUX_CB_AUDIT: selinux_audit = cb.func_audit; @@ -106,7 +108,7 @@ selinux_get_callback(int type) switch (type) { case SELINUX_CB_LOG: - cb.func_log = selinux_log; + cb.func_log = selinux_log_direct; break; case SELINUX_CB_AUDIT: cb.func_audit = selinux_audit; diff --git a/libselinux/src/callbacks.h b/libselinux/src/callbacks.h index 03d87f0c..f4dab157 100644 --- a/libselinux/src/callbacks.h +++ b/libselinux/src/callbacks.h @@ -10,9 +10,11 @@ #include <string.h> #include <selinux/selinux.h> +#include "selinux_internal.h" + /* callback pointers */ extern int __attribute__ ((format(printf, 2, 3))) -(*selinux_log) (int type, const char *, ...) ; +(*selinux_log_direct) (int type, const char *, ...) ; extern int (*selinux_audit) (void *, security_class_t, char *, size_t) ; @@ -26,4 +28,13 @@ extern int extern int (*selinux_netlink_policyload) (int seqno) ; +/* Thread-safe selinux_log() function */ +extern pthread_mutex_t log_mutex; + +#define selinux_log(type, ...) do { \ + __pthread_mutex_lock(&log_mutex); \ + selinux_log_direct(type, __VA_ARGS__); \ + __pthread_mutex_unlock(&log_mutex); \ +} while(0) + #endif /* _SELINUX_CALLBACKS_H_ */ -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH userspace v2 4/6] selinux_restorecon: add a global mutex to synchronize progress output 2021-10-14 14:53 [PATCH userspace v2 0/6] Parallel setfiles/restorecon Ondrej Mosnacek ` (2 preceding siblings ...) 2021-10-14 14:53 ` [PATCH userspace v2 3/6] libselinux: make selinux_log() thread-safe Ondrej Mosnacek @ 2021-10-14 14:53 ` Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 5/6] selinux_restorecon: introduce selinux_restorecon_parallel(3) Ondrej Mosnacek ` (2 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Ondrej Mosnacek @ 2021-10-14 14:53 UTC (permalink / raw) To: selinux 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 732f0ccc..252b775f 100644 --- a/libselinux/src/selinux_restorecon.c +++ b/libselinux/src/selinux_restorecon.c @@ -60,6 +60,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. */ static struct dir_xattr *dir_xattr_list; @@ -647,6 +648,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) { @@ -658,6 +660,7 @@ static int restorecon_sb(const char *pathname, const struct stat *sb, } fflush(stdout); } + __pthread_mutex_unlock(&progress_mutex); } if (flags->add_assoc) { -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH userspace v2 5/6] selinux_restorecon: introduce selinux_restorecon_parallel(3) 2021-10-14 14:53 [PATCH userspace v2 0/6] Parallel setfiles/restorecon Ondrej Mosnacek ` (3 preceding siblings ...) 2021-10-14 14:53 ` [PATCH userspace v2 4/6] selinux_restorecon: add a global mutex to synchronize progress output Ondrej Mosnacek @ 2021-10-14 14:53 ` Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 6/6] setfiles/restorecon: support parallel relabeling Ondrej Mosnacek 2021-10-15 12:37 ` [PATCH userspace v2 0/6] Parallel setfiles/restorecon Christian Göttsche 6 siblings, 0 replies; 11+ messages in thread From: Ondrej Mosnacek @ 2021-10-14 14:53 UTC (permalink / raw) To: selinux 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 | 444 ++++++++++++------ libselinux/src/selinuxswig_python.i | 6 +- libselinux/src/selinuxswig_python_exception.i | 8 + 8 files changed, 368 insertions(+), 153 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..d138e951 100644 --- a/libselinux/src/libselinux.map +++ b/libselinux/src/libselinux.map @@ -240,3 +240,8 @@ LIBSELINUX_1.0 { local: *; }; + +LIBSELINUX_3.3 { + 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 252b775f..f5af7001 100644 --- a/libselinux/src/selinux_restorecon.c +++ b/libselinux/src/selinux_restorecon.c @@ -610,7 +610,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; @@ -639,7 +639,7 @@ static int restorecon_sb(const char *pathname, const struct stat *sb, sb->st_mode); if (rc < 0) { - if (errno == ENOENT && flags->warnonnomatch) + if (errno == ENOENT && flags->warnonnomatch && first) selinux_log(SELINUX_INFO, "Warning no default label for %s\n", lookup_path); @@ -668,7 +668,8 @@ static int restorecon_sb(const char *pathname, const struct stat *sb, if (rc < 0) { selinux_log(SELINUX_ERROR, - "filespec_add error: %s\n", pathname); + "filespec_add error: %s\n", + pathname); freecon(newcon); return -1; } @@ -681,8 +682,7 @@ 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(SELINUX_INFO, "%s matched by %s\n", pathname, newcon); if (lgetfilecon_raw(pathname, &curcon) < 0) { if (errno != ENODATA) @@ -697,7 +697,7 @@ static int restorecon_sb(const char *pathname, const struct stat *sb, if (flags->verbose) { selinux_log(SELINUX_INFO, "%s not reset as customized by admin to %s\n", - pathname, curcon); + pathname, curcon); } goto out; } @@ -814,66 +814,223 @@ 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 (((void)(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; - flags.nochange = (restorecon_flags & + switch (ftsent->fts_info) { + case FTS_DC: + selinux_log(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: + error = errno; + errno = ftsent->fts_errno; + selinux_log(SELINUX_ERROR, + "Could not read %s: %m.\n", + ftsent->fts_path); + errno = error; + fts_set(fts, ftsent, FTS_SKIP); + continue; + case FTS_NS: + error = errno; + errno = ftsent->fts_errno; + selinux_log(SELINUX_ERROR, + "Could not stat %s: %m.\n", + ftsent->fts_path); + errno = error; + fts_set(fts, ftsent, FTS_SKIP); + continue; + case FTS_ERR: + error = errno; + errno = ftsent->fts_errno; + selinux_log(SELINUX_ERROR, + "Error on %s: %m.\n", + ftsent->fts_path); + errno = error; + 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(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; + + 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; - int errno_tmp; - 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); @@ -886,13 +1043,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; @@ -937,7 +1112,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; @@ -952,21 +1127,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: %m\n", pathname); @@ -975,21 +1150,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; /* @@ -1001,106 +1176,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; - - 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; + state.dev_num = state.ftsent_first->fts_statp->st_dev; - 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: - errno_tmp = errno; - errno = ftsent->fts_errno; - selinux_log(SELINUX_ERROR, - "Could not read %s: %m.\n", - ftsent->fts_path); - errno = errno_tmp; - fts_set(fts, ftsent, FTS_SKIP); - continue; - case FTS_NS: - errno_tmp = errno; - errno = ftsent->fts_errno; - selinux_log(SELINUX_ERROR, - "Could not stat %s: %m.\n", - ftsent->fts_path); - errno = errno_tmp; - fts_set(fts, ftsent, FTS_SKIP); - continue; - case FTS_ERR: - errno_tmp = errno; - errno = ftsent->fts_errno; - selinux_log(SELINUX_ERROR, - "Error on %s: %m.\n", - ftsent->fts_path); - errno = errno_tmp; - 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, @@ -1115,22 +1238,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; @@ -1164,6 +1286,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) { diff --git a/libselinux/src/selinuxswig_python.i b/libselinux/src/selinuxswig_python.i index 4c73bf92..17e03b9e 100644 --- a/libselinux/src/selinuxswig_python.i +++ b/libselinux/src/selinuxswig_python.i @@ -20,7 +20,7 @@ DISABLED = -1 PERMISSIVE = 0 ENFORCING = 1 -def restorecon(path, recursive=False, verbose=False, force=False): +def restorecon(path, recursive=False, verbose=False, force=False, nthreads=1): """ Restore SELinux context on a given path Arguments: @@ -32,6 +32,8 @@ def restorecon(path, recursive=False, verbose=False, force=False): force -- Force reset of context to match file_context for customizable files, and the default file context, changing the user, role, range portion as well as the type (default False) + nthreads -- The number of threads to use during relabeling, or 0 to use as many + threads as there are online CPU cores (default 1) """ restorecon_flags = SELINUX_RESTORECON_IGNORE_DIGEST | SELINUX_RESTORECON_REALPATH @@ -41,7 +43,7 @@ def restorecon(path, recursive=False, verbose=False, force=False): restorecon_flags |= SELINUX_RESTORECON_VERBOSE if force: restorecon_flags |= SELINUX_RESTORECON_SET_SPECFILE_CTX - selinux_restorecon(os.path.expanduser(path), restorecon_flags) + selinux_restorecon_parallel(os.path.expanduser(path), restorecon_flags, nthreads) def chcon(path, context, recursive=False): """ Set the SELinux context on a given path """ diff --git a/libselinux/src/selinuxswig_python_exception.i b/libselinux/src/selinuxswig_python_exception.i index 237ea69a..a02f4923 100644 --- a/libselinux/src/selinuxswig_python_exception.i +++ b/libselinux/src/selinuxswig_python_exception.i @@ -1183,6 +1183,14 @@ } } +%exception selinux_restorecon_parallel { + $action + if (result < 0) { + PyErr_SetFromErrno(PyExc_OSError); + SWIG_fail; + } +} + %exception selinux_restorecon_set_alt_rootpath { $action if (result < 0) { -- 2.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH userspace v2 6/6] setfiles/restorecon: support parallel relabeling 2021-10-14 14:53 [PATCH userspace v2 0/6] Parallel setfiles/restorecon Ondrej Mosnacek ` (4 preceding siblings ...) 2021-10-14 14:53 ` [PATCH userspace v2 5/6] selinux_restorecon: introduce selinux_restorecon_parallel(3) Ondrej Mosnacek @ 2021-10-14 14:53 ` Ondrej Mosnacek 2021-10-15 12:37 ` [PATCH userspace v2 0/6] Parallel setfiles/restorecon Christian Göttsche 6 siblings, 0 replies; 11+ messages in thread From: Ondrej Mosnacek @ 2021-10-14 14:53 UTC (permalink / raw) To: selinux 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 real 0m21.689s user 0m21.070s sys 0m0.494s WITH PATCHES: $ time restorecon -rn /usr real 0m23.940s user 0m23.127s sys 0m0.653s $ time restorecon -rn -T 2 /usr real 0m13.145s user 0m25.306s sys 0m0.695s $ time restorecon -rn -T 4 /usr real 0m7.559s user 0m28.470s sys 0m1.099s $ time restorecon -rn -T 8 /usr real 0m5.186s user 0m37.450s sys 0m2.094s $ time restorecon -rn -T 16 /usr real 0m3.831s user 0m51.220s sys 0m4.895s $ time restorecon -rn -T 32 /usr real 0m2.650s user 1m5.136s sys 0m6.614s 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 63d81850..d7670a8f 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.31.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH userspace v2 0/6] Parallel setfiles/restorecon 2021-10-14 14:53 [PATCH userspace v2 0/6] Parallel setfiles/restorecon Ondrej Mosnacek ` (5 preceding siblings ...) 2021-10-14 14:53 ` [PATCH userspace v2 6/6] setfiles/restorecon: support parallel relabeling Ondrej Mosnacek @ 2021-10-15 12:37 ` Christian Göttsche 2021-10-15 13:25 ` Ondrej Mosnacek 6 siblings, 1 reply; 11+ messages in thread From: Christian Göttsche @ 2021-10-15 12:37 UTC (permalink / raw) To: Ondrej Mosnacek; +Cc: SElinux list On Thu, 14 Oct 2021 at 16:53, Ondrej Mosnacek <omosnace@redhat.com> 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. > > 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. > > Changes v1->v2: > - make selinux_log() synchronized instead of introducing selinux_log_sync() > - fix -Wcomma warning > - update the swig files as well > - bump new symbol version to LIBSELINUX_3.3 (this may need further update > depending on when this gets merged) > > Ondrej Mosnacek (6): > selinux_restorecon: simplify fl_head allocation by using calloc() > selinux_restorecon: protect file_spec list with a mutex > libselinux: make selinux_log() thread-safe > 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/callbacks.c | 8 +- > libselinux/src/callbacks.h | 13 +- > libselinux/src/libselinux.map | 5 + > libselinux/src/selinux_internal.h | 14 + > libselinux/src/selinux_restorecon.c | 466 ++++++++++++------ > libselinux/src/selinuxswig_python.i | 6 +- > libselinux/src/selinuxswig_python_exception.i | 8 + > 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 +- > 16 files changed, 444 insertions(+), 177 deletions(-) > create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3 > > -- > 2.31.1 > Running under ThreadSanitizer shows multiple instances of the following issue: ================== WARNING: ThreadSanitizer: data race (pid=16933) Read of size 4 at 0x7f8790d636f4 by thread T2: #0 lookup_all ./libselinux/src/label_file.c:954 (libselinux.so.1+0x14d1b) #1 lookup_common ./libselinux/src/label_file.c:997 (libselinux.so.1+0x14f62) #2 lookup ./libselinux/src/label_file.c:1095 (libselinux.so.1+0x14fff) #3 selabel_lookup_common ./libselinux/src/label.c:167 (libselinux.so.1+0x12291) #4 selabel_lookup_raw ./libselinux/src/label.c:256 (libselinux.so.1+0x126ca) #5 restorecon_sb ./libselinux/src/selinux_restorecon.c:638 (libselinux.so.1+0x20c76) #6 selinux_restorecon_thread ./libselinux/src/selinux_restorecon.c:947 (libselinux.so.1+0x21e99) Previous write of size 4 at 0x7f8790d636f4 by thread T1: #0 lookup_all ./libselinux/src/label_file.c:954 (libselinux.so.1+0x14d29) #1 lookup_common ./libselinux/src/label_file.c:997 (libselinux.so.1+0x14f62) #2 lookup ./libselinux/src/label_file.c:1095 (libselinux.so.1+0x14fff) #3 selabel_lookup_common ./libselinux/src/label.c:167 (libselinux.so.1+0x12291) #4 selabel_lookup_raw ./libselinux/src/label.c:256 (libselinux.so.1+0x126ca) #5 restorecon_sb ./libselinux/src/selinux_restorecon.c:638 (libselinux.so.1+0x20c76) #6 selinux_restorecon_thread ./libselinux/src/selinux_restorecon.c:947 (libselinux.so.1+0x21e99) Location is heap block of size 267120 at 0x7f8790d45000 allocated by main thread: #0 malloc ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:655 (libtsan.so.0+0x31799) #1 sort_specs ./libselinux/src/label_file.h:207 (libselinux.so.1+0x17bd9) #2 init ./libselinux/src/label_file.c:795 (libselinux.so.1+0x17bd9) #3 selabel_file_init ./libselinux/src/label_file.c:1293 (libselinux.so.1+0x1835b) #4 selabel_open ./libselinux/src/label.c:228 (libselinux.so.1+0x125d8) #5 restore_init ./policycoreutils/setfiles/restore.c:30 (setfiles+0x3886) #6 main ./policycoreutils/setfiles/setfiles.c:434 (setfiles+0x344e) Thread T2 (tid=16940, running) created by main thread at: #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 (libtsan.so.0+0x5ad75) #1 selinux_restorecon_common ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) #2 selinux_restorecon_parallel ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) Thread T1 (tid=16939, running) created by main thread at: #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 (libtsan.so.0+0x5ad75) #1 selinux_restorecon_common ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) #2 selinux_restorecon_parallel ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) SUMMARY: ThreadSanitizer: data race ./libselinux/src/label_file.c:954 in lookup_all ================== Also some thread joins seems to be missing: ================== WARNING: ThreadSanitizer: thread leak (pid=16933) Thread T1 (tid=16939, finished) created by main thread at: #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 (libtsan.so.0+0x5ad75) #1 selinux_restorecon_common ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) #2 selinux_restorecon_parallel ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) And 2 more similar thread leaks. SUMMARY: ThreadSanitizer: thread leak ./libselinux/src/selinux_restorecon.c:1197 in selinux_restorecon_common ================== Performance on 4 cores: real 0m27.323s user 0m20.824s sys 0m4.029s vs real 0m12.462s user 0m31.220s sys 0m4.316s ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH userspace v2 0/6] Parallel setfiles/restorecon 2021-10-15 12:37 ` [PATCH userspace v2 0/6] Parallel setfiles/restorecon Christian Göttsche @ 2021-10-15 13:25 ` Ondrej Mosnacek 2021-10-18 8:56 ` Ondrej Mosnacek 0 siblings, 1 reply; 11+ messages in thread From: Ondrej Mosnacek @ 2021-10-15 13:25 UTC (permalink / raw) To: Christian Göttsche; +Cc: SElinux list On Fri, Oct 15, 2021 at 2:37 PM Christian Göttsche <cgzones@googlemail.com> wrote: > On Thu, 14 Oct 2021 at 16:53, Ondrej Mosnacek <omosnace@redhat.com> 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. > > > > 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. > > > > Changes v1->v2: > > - make selinux_log() synchronized instead of introducing selinux_log_sync() > > - fix -Wcomma warning > > - update the swig files as well > > - bump new symbol version to LIBSELINUX_3.3 (this may need further update > > depending on when this gets merged) > > > > Ondrej Mosnacek (6): > > selinux_restorecon: simplify fl_head allocation by using calloc() > > selinux_restorecon: protect file_spec list with a mutex > > libselinux: make selinux_log() thread-safe > > 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/callbacks.c | 8 +- > > libselinux/src/callbacks.h | 13 +- > > libselinux/src/libselinux.map | 5 + > > libselinux/src/selinux_internal.h | 14 + > > libselinux/src/selinux_restorecon.c | 466 ++++++++++++------ > > libselinux/src/selinuxswig_python.i | 6 +- > > libselinux/src/selinuxswig_python_exception.i | 8 + > > 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 +- > > 16 files changed, 444 insertions(+), 177 deletions(-) > > create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3 > > > > -- > > 2.31.1 > > > > > Running under ThreadSanitizer shows multiple instances of the following issue: > > ================== > WARNING: ThreadSanitizer: data race (pid=16933) > Read of size 4 at 0x7f8790d636f4 by thread T2: > #0 lookup_all ./libselinux/src/label_file.c:954 (libselinux.so.1+0x14d1b) > #1 lookup_common ./libselinux/src/label_file.c:997 (libselinux.so.1+0x14f62) > #2 lookup ./libselinux/src/label_file.c:1095 (libselinux.so.1+0x14fff) > #3 selabel_lookup_common ./libselinux/src/label.c:167 > (libselinux.so.1+0x12291) > #4 selabel_lookup_raw ./libselinux/src/label.c:256 (libselinux.so.1+0x126ca) > #5 restorecon_sb ./libselinux/src/selinux_restorecon.c:638 > (libselinux.so.1+0x20c76) > #6 selinux_restorecon_thread > ./libselinux/src/selinux_restorecon.c:947 (libselinux.so.1+0x21e99) > > Previous write of size 4 at 0x7f8790d636f4 by thread T1: > #0 lookup_all ./libselinux/src/label_file.c:954 (libselinux.so.1+0x14d29) > #1 lookup_common ./libselinux/src/label_file.c:997 (libselinux.so.1+0x14f62) > #2 lookup ./libselinux/src/label_file.c:1095 (libselinux.so.1+0x14fff) > #3 selabel_lookup_common ./libselinux/src/label.c:167 > (libselinux.so.1+0x12291) > #4 selabel_lookup_raw ./libselinux/src/label.c:256 (libselinux.so.1+0x126ca) > #5 restorecon_sb ./libselinux/src/selinux_restorecon.c:638 > (libselinux.so.1+0x20c76) > #6 selinux_restorecon_thread > ./libselinux/src/selinux_restorecon.c:947 (libselinux.so.1+0x21e99) > > Location is heap block of size 267120 at 0x7f8790d45000 allocated by > main thread: > #0 malloc ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:655 > (libtsan.so.0+0x31799) > #1 sort_specs ./libselinux/src/label_file.h:207 (libselinux.so.1+0x17bd9) > #2 init ./libselinux/src/label_file.c:795 (libselinux.so.1+0x17bd9) > #3 selabel_file_init ./libselinux/src/label_file.c:1293 > (libselinux.so.1+0x1835b) > #4 selabel_open ./libselinux/src/label.c:228 (libselinux.so.1+0x125d8) > #5 restore_init ./policycoreutils/setfiles/restore.c:30 (setfiles+0x3886) > #6 main ./policycoreutils/setfiles/setfiles.c:434 (setfiles+0x344e) > > Thread T2 (tid=16940, running) created by main thread at: > #0 pthread_create > ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 > (libtsan.so.0+0x5ad75) > #1 selinux_restorecon_common > ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) > #2 selinux_restorecon_parallel > ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) > #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) > #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) > > Thread T1 (tid=16939, running) created by main thread at: > #0 pthread_create > ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 > (libtsan.so.0+0x5ad75) > #1 selinux_restorecon_common > ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) > #2 selinux_restorecon_parallel > ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) > #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) > #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) > > SUMMARY: ThreadSanitizer: data race ./libselinux/src/label_file.c:954 > in lookup_all > ================== Good catch, although this one seems to be pre-existing w.r.t. thread safety of selinux_restorecon(3) (which seems to be implied by the existence of struct spec::regex_lock). I spotted some existing usage of GCC atomic builtins, so I'll try to fix it using those. > Also some thread joins seems to be missing: > > ================== > WARNING: ThreadSanitizer: thread leak (pid=16933) > Thread T1 (tid=16939, finished) created by main thread at: > #0 pthread_create > ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 > (libtsan.so.0+0x5ad75) > #1 selinux_restorecon_common > ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) > #2 selinux_restorecon_parallel > ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) > #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) > #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) > > And 2 more similar thread leaks. > > SUMMARY: ThreadSanitizer: thread leak > ./libselinux/src/selinux_restorecon.c:1197 in > selinux_restorecon_common > ================== Hm... My intention was to avoid the need for joins by synchronizing via other means and just letting the threads exit on their own, but I guess that doesn't fly with the sanitizer. Guess I'll have to allocate an array for the handles and join the threads at the end after all... -- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH userspace v2 0/6] Parallel setfiles/restorecon 2021-10-15 13:25 ` Ondrej Mosnacek @ 2021-10-18 8:56 ` Ondrej Mosnacek 2021-10-19 12:29 ` Ondrej Mosnacek 0 siblings, 1 reply; 11+ messages in thread From: Ondrej Mosnacek @ 2021-10-18 8:56 UTC (permalink / raw) To: Christian Göttsche; +Cc: SElinux list On Fri, Oct 15, 2021 at 3:25 PM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Fri, Oct 15, 2021 at 2:37 PM Christian Göttsche > <cgzones@googlemail.com> wrote: > > On Thu, 14 Oct 2021 at 16:53, Ondrej Mosnacek <omosnace@redhat.com> 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. > > > > > > 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. > > > > > > Changes v1->v2: > > > - make selinux_log() synchronized instead of introducing selinux_log_sync() > > > - fix -Wcomma warning > > > - update the swig files as well > > > - bump new symbol version to LIBSELINUX_3.3 (this may need further update > > > depending on when this gets merged) > > > > > > Ondrej Mosnacek (6): > > > selinux_restorecon: simplify fl_head allocation by using calloc() > > > selinux_restorecon: protect file_spec list with a mutex > > > libselinux: make selinux_log() thread-safe > > > 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/callbacks.c | 8 +- > > > libselinux/src/callbacks.h | 13 +- > > > libselinux/src/libselinux.map | 5 + > > > libselinux/src/selinux_internal.h | 14 + > > > libselinux/src/selinux_restorecon.c | 466 ++++++++++++------ > > > libselinux/src/selinuxswig_python.i | 6 +- > > > libselinux/src/selinuxswig_python_exception.i | 8 + > > > 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 +- > > > 16 files changed, 444 insertions(+), 177 deletions(-) > > > create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3 > > > > > > -- > > > 2.31.1 > > > > > > > > > Running under ThreadSanitizer shows multiple instances of the following issue: > > > > ================== > > WARNING: ThreadSanitizer: data race (pid=16933) > > Read of size 4 at 0x7f8790d636f4 by thread T2: > > #0 lookup_all ./libselinux/src/label_file.c:954 (libselinux.so.1+0x14d1b) > > #1 lookup_common ./libselinux/src/label_file.c:997 (libselinux.so.1+0x14f62) > > #2 lookup ./libselinux/src/label_file.c:1095 (libselinux.so.1+0x14fff) > > #3 selabel_lookup_common ./libselinux/src/label.c:167 > > (libselinux.so.1+0x12291) > > #4 selabel_lookup_raw ./libselinux/src/label.c:256 (libselinux.so.1+0x126ca) > > #5 restorecon_sb ./libselinux/src/selinux_restorecon.c:638 > > (libselinux.so.1+0x20c76) > > #6 selinux_restorecon_thread > > ./libselinux/src/selinux_restorecon.c:947 (libselinux.so.1+0x21e99) > > > > Previous write of size 4 at 0x7f8790d636f4 by thread T1: > > #0 lookup_all ./libselinux/src/label_file.c:954 (libselinux.so.1+0x14d29) > > #1 lookup_common ./libselinux/src/label_file.c:997 (libselinux.so.1+0x14f62) > > #2 lookup ./libselinux/src/label_file.c:1095 (libselinux.so.1+0x14fff) > > #3 selabel_lookup_common ./libselinux/src/label.c:167 > > (libselinux.so.1+0x12291) > > #4 selabel_lookup_raw ./libselinux/src/label.c:256 (libselinux.so.1+0x126ca) > > #5 restorecon_sb ./libselinux/src/selinux_restorecon.c:638 > > (libselinux.so.1+0x20c76) > > #6 selinux_restorecon_thread > > ./libselinux/src/selinux_restorecon.c:947 (libselinux.so.1+0x21e99) > > > > Location is heap block of size 267120 at 0x7f8790d45000 allocated by > > main thread: > > #0 malloc ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:655 > > (libtsan.so.0+0x31799) > > #1 sort_specs ./libselinux/src/label_file.h:207 (libselinux.so.1+0x17bd9) > > #2 init ./libselinux/src/label_file.c:795 (libselinux.so.1+0x17bd9) > > #3 selabel_file_init ./libselinux/src/label_file.c:1293 > > (libselinux.so.1+0x1835b) > > #4 selabel_open ./libselinux/src/label.c:228 (libselinux.so.1+0x125d8) > > #5 restore_init ./policycoreutils/setfiles/restore.c:30 (setfiles+0x3886) > > #6 main ./policycoreutils/setfiles/setfiles.c:434 (setfiles+0x344e) > > > > Thread T2 (tid=16940, running) created by main thread at: > > #0 pthread_create > > ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 > > (libtsan.so.0+0x5ad75) > > #1 selinux_restorecon_common > > ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) > > #2 selinux_restorecon_parallel > > ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) > > #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) > > #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) > > > > Thread T1 (tid=16939, running) created by main thread at: > > #0 pthread_create > > ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 > > (libtsan.so.0+0x5ad75) > > #1 selinux_restorecon_common > > ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) > > #2 selinux_restorecon_parallel > > ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) > > #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) > > #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) > > > > SUMMARY: ThreadSanitizer: data race ./libselinux/src/label_file.c:954 > > in lookup_all > > ================== > > Good catch, although this one seems to be pre-existing w.r.t. thread > safety of selinux_restorecon(3) (which seems to be implied by the > existence of struct spec::regex_lock). I spotted some existing usage > of GCC atomic builtins, so I'll try to fix it using those. > > > Also some thread joins seems to be missing: > > > > ================== > > WARNING: ThreadSanitizer: thread leak (pid=16933) > > Thread T1 (tid=16939, finished) created by main thread at: > > #0 pthread_create > > ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 > > (libtsan.so.0+0x5ad75) > > #1 selinux_restorecon_common > > ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) > > #2 selinux_restorecon_parallel > > ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) > > #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) > > #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) > > > > And 2 more similar thread leaks. > > > > SUMMARY: ThreadSanitizer: thread leak > > ./libselinux/src/selinux_restorecon.c:1197 in > > selinux_restorecon_common > > ================== > > Hm... My intention was to avoid the need for joins by synchronizing > via other means and just letting the threads exit on their own, but I > guess that doesn't fly with the sanitizer. Guess I'll have to allocate > an array for the handles and join the threads at the end after all... So I was able to get ThreadSanitizer to detect the race (by using only two threads and a very small number of files), but then the program just runs forever and I never get to see the thread leaks... Any hints on how you ran/compiled it to see the thread leaks? Or how long you had to wait for the program to finish? BTW, I only managed to get thread sanitizer to work with GCC. With CLang I always got some linker error... -- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH userspace v2 0/6] Parallel setfiles/restorecon 2021-10-18 8:56 ` Ondrej Mosnacek @ 2021-10-19 12:29 ` Ondrej Mosnacek 0 siblings, 0 replies; 11+ messages in thread From: Ondrej Mosnacek @ 2021-10-19 12:29 UTC (permalink / raw) To: Christian Göttsche; +Cc: SElinux list On Mon, Oct 18, 2021 at 10:56 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > On Fri, Oct 15, 2021 at 3:25 PM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Fri, Oct 15, 2021 at 2:37 PM Christian Göttsche > > <cgzones@googlemail.com> wrote: > > > On Thu, 14 Oct 2021 at 16:53, Ondrej Mosnacek <omosnace@redhat.com> 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. > > > > > > > > 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. > > > > > > > > Changes v1->v2: > > > > - make selinux_log() synchronized instead of introducing selinux_log_sync() > > > > - fix -Wcomma warning > > > > - update the swig files as well > > > > - bump new symbol version to LIBSELINUX_3.3 (this may need further update > > > > depending on when this gets merged) > > > > > > > > Ondrej Mosnacek (6): > > > > selinux_restorecon: simplify fl_head allocation by using calloc() > > > > selinux_restorecon: protect file_spec list with a mutex > > > > libselinux: make selinux_log() thread-safe > > > > 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/callbacks.c | 8 +- > > > > libselinux/src/callbacks.h | 13 +- > > > > libselinux/src/libselinux.map | 5 + > > > > libselinux/src/selinux_internal.h | 14 + > > > > libselinux/src/selinux_restorecon.c | 466 ++++++++++++------ > > > > libselinux/src/selinuxswig_python.i | 6 +- > > > > libselinux/src/selinuxswig_python_exception.i | 8 + > > > > 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 +- > > > > 16 files changed, 444 insertions(+), 177 deletions(-) > > > > create mode 100644 libselinux/man/man3/selinux_restorecon_parallel.3 > > > > > > > > -- > > > > 2.31.1 > > > > > > > > > > > > > Running under ThreadSanitizer shows multiple instances of the following issue: > > > > > > ================== > > > WARNING: ThreadSanitizer: data race (pid=16933) > > > Read of size 4 at 0x7f8790d636f4 by thread T2: > > > #0 lookup_all ./libselinux/src/label_file.c:954 (libselinux.so.1+0x14d1b) > > > #1 lookup_common ./libselinux/src/label_file.c:997 (libselinux.so.1+0x14f62) > > > #2 lookup ./libselinux/src/label_file.c:1095 (libselinux.so.1+0x14fff) > > > #3 selabel_lookup_common ./libselinux/src/label.c:167 > > > (libselinux.so.1+0x12291) > > > #4 selabel_lookup_raw ./libselinux/src/label.c:256 (libselinux.so.1+0x126ca) > > > #5 restorecon_sb ./libselinux/src/selinux_restorecon.c:638 > > > (libselinux.so.1+0x20c76) > > > #6 selinux_restorecon_thread > > > ./libselinux/src/selinux_restorecon.c:947 (libselinux.so.1+0x21e99) > > > > > > Previous write of size 4 at 0x7f8790d636f4 by thread T1: > > > #0 lookup_all ./libselinux/src/label_file.c:954 (libselinux.so.1+0x14d29) > > > #1 lookup_common ./libselinux/src/label_file.c:997 (libselinux.so.1+0x14f62) > > > #2 lookup ./libselinux/src/label_file.c:1095 (libselinux.so.1+0x14fff) > > > #3 selabel_lookup_common ./libselinux/src/label.c:167 > > > (libselinux.so.1+0x12291) > > > #4 selabel_lookup_raw ./libselinux/src/label.c:256 (libselinux.so.1+0x126ca) > > > #5 restorecon_sb ./libselinux/src/selinux_restorecon.c:638 > > > (libselinux.so.1+0x20c76) > > > #6 selinux_restorecon_thread > > > ./libselinux/src/selinux_restorecon.c:947 (libselinux.so.1+0x21e99) > > > > > > Location is heap block of size 267120 at 0x7f8790d45000 allocated by > > > main thread: > > > #0 malloc ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:655 > > > (libtsan.so.0+0x31799) > > > #1 sort_specs ./libselinux/src/label_file.h:207 (libselinux.so.1+0x17bd9) > > > #2 init ./libselinux/src/label_file.c:795 (libselinux.so.1+0x17bd9) > > > #3 selabel_file_init ./libselinux/src/label_file.c:1293 > > > (libselinux.so.1+0x1835b) > > > #4 selabel_open ./libselinux/src/label.c:228 (libselinux.so.1+0x125d8) > > > #5 restore_init ./policycoreutils/setfiles/restore.c:30 (setfiles+0x3886) > > > #6 main ./policycoreutils/setfiles/setfiles.c:434 (setfiles+0x344e) > > > > > > Thread T2 (tid=16940, running) created by main thread at: > > > #0 pthread_create > > > ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 > > > (libtsan.so.0+0x5ad75) > > > #1 selinux_restorecon_common > > > ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) > > > #2 selinux_restorecon_parallel > > > ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) > > > #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) > > > #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) > > > > > > Thread T1 (tid=16939, running) created by main thread at: > > > #0 pthread_create > > > ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 > > > (libtsan.so.0+0x5ad75) > > > #1 selinux_restorecon_common > > > ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) > > > #2 selinux_restorecon_parallel > > > ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) > > > #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) > > > #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) > > > > > > SUMMARY: ThreadSanitizer: data race ./libselinux/src/label_file.c:954 > > > in lookup_all > > > ================== > > > > Good catch, although this one seems to be pre-existing w.r.t. thread > > safety of selinux_restorecon(3) (which seems to be implied by the > > existence of struct spec::regex_lock). I spotted some existing usage > > of GCC atomic builtins, so I'll try to fix it using those. > > > > > Also some thread joins seems to be missing: > > > > > > ================== > > > WARNING: ThreadSanitizer: thread leak (pid=16933) > > > Thread T1 (tid=16939, finished) created by main thread at: > > > #0 pthread_create > > > ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:969 > > > (libtsan.so.0+0x5ad75) > > > #1 selinux_restorecon_common > > > ./libselinux/src/selinux_restorecon.c:1197 (libselinux.so.1+0x2341c) > > > #2 selinux_restorecon_parallel > > > ./libselinux/src/selinux_restorecon.c:1306 (libselinux.so.1+0x23985) > > > #3 process_glob ./policycoreutils/setfiles/restore.c:94 (setfiles+0x3ba4) > > > #4 main ./policycoreutils/setfiles/setfiles.c:463 (setfiles+0x362a) > > > > > > And 2 more similar thread leaks. > > > > > > SUMMARY: ThreadSanitizer: thread leak > > > ./libselinux/src/selinux_restorecon.c:1197 in > > > selinux_restorecon_common > > > ================== > > > > Hm... My intention was to avoid the need for joins by synchronizing > > via other means and just letting the threads exit on their own, but I > > guess that doesn't fly with the sanitizer. Guess I'll have to allocate > > an array for the handles and join the threads at the end after all... > > So I was able to get ThreadSanitizer to detect the race (by using only > two threads and a very small number of files), but then the program > just runs forever and I never get to see the thread leaks... Any hints > on how you ran/compiled it to see the thread leaks? Or how long you > had to wait for the program to finish? BTW, I only managed to get > thread sanitizer to work with GCC. With CLang I always got some linker > error... Seems it was getting stuck because of the lack of joins. Once I rewrote selinux_restorecon_common() to wait for the threads via pthread_join(), setfiles built with -fsanitize=thread exited quickly and without warnings. -- Ondrej Mosnacek Software Engineer, Linux Security - SELinux kernel Red Hat, Inc. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-19 12:29 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-14 14:53 [PATCH userspace v2 0/6] Parallel setfiles/restorecon Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 1/6] selinux_restorecon: simplify fl_head allocation by using calloc() Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 2/6] selinux_restorecon: protect file_spec list with a mutex Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 3/6] libselinux: make selinux_log() thread-safe Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 4/6] selinux_restorecon: add a global mutex to synchronize progress output Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 5/6] selinux_restorecon: introduce selinux_restorecon_parallel(3) Ondrej Mosnacek 2021-10-14 14:53 ` [PATCH userspace v2 6/6] setfiles/restorecon: support parallel relabeling Ondrej Mosnacek 2021-10-15 12:37 ` [PATCH userspace v2 0/6] Parallel setfiles/restorecon Christian Göttsche 2021-10-15 13:25 ` Ondrej Mosnacek 2021-10-18 8:56 ` Ondrej Mosnacek 2021-10-19 12:29 ` Ondrej Mosnacek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.