On Fri, Oct 15, 2021 at 3:25 PM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > On Fri, Oct 15, 2021 at 2:37 PM Christian Göttsche > <cgzones@xxxxxxxxxxxxxx> wrote: > > On Thu, 14 Oct 2021 at 16:53, Ondrej Mosnacek <omosnace@xxxxxxxxxx> 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.
