From 24837d953f0f506cb8e4484911b30305e2e1a589 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Fri, 26 Aug 2022 18:36:13 +0200 Subject: [PATCH] Related: rhbz#1978119 - [Improvement] avoid interlocking among threads that use `libsss_nss_idmap` API (or other sss_client libs) --- 0010-CLIENT-fix-client-fd-leak.patch | 295 +++++++++++++++++++++++++++ sssd.spec | 6 +- 2 files changed, 300 insertions(+), 1 deletion(-) create mode 100644 0010-CLIENT-fix-client-fd-leak.patch diff --git a/0010-CLIENT-fix-client-fd-leak.patch b/0010-CLIENT-fix-client-fd-leak.patch new file mode 100644 index 0000000..48622c8 --- /dev/null +++ b/0010-CLIENT-fix-client-fd-leak.patch @@ -0,0 +1,295 @@ +From 1b2e4760c52b9abd0d9b9f35b47ed72e79922ccc Mon Sep 17 00:00:00 2001 +From: Alexey Tikhonov +Date: Thu, 25 Aug 2022 18:10:46 +0200 +Subject: [PATCH] CLIENT: fix client fd leak + + - close client socket at thread exit + - only build lock-free client support if libc has required + functionality for a proper cleanup + - use proper mechanisms to init lock_mode only once + +:relnote:Lock-free client support will be only built if libc +provides `pthread_key_create()` and `pthread_once()`. For glibc +this means version 2.34+ + +Reviewed-by: Justin Stephenson +Reviewed-by: Sumit Bose +(cherry picked from commit 1a6f67c92399ff8e358a6c6cdda43fb2547a5fdb) +--- + configure.ac | 29 +++++++++-- + src/man/Makefile.am | 5 +- + src/man/sssd.8.xml | 2 +- + src/sss_client/common.c | 83 +++++++++++++++++++------------- + src/sss_client/idmap/common_ex.c | 4 ++ + 5 files changed, 84 insertions(+), 39 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 93bd93b85..5a05de41e 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -51,18 +51,39 @@ AC_CHECK_TYPES([errno_t], [], [], [[#include ]]) + m4_include([src/build_macros.m4]) + BUILD_WITH_SHARED_BUILD_DIR + +-AC_COMPILE_IFELSE( ++ ++SAVE_LIBS=$LIBS ++LIBS= ++AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[#include ]], + [[pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER; +- (void) m; /* unused */ ++ pthread_mutex_lock(&m); ++ pthread_mutex_unlock(&m); + ]])], + [AC_DEFINE([HAVE_PTHREAD], [1], [Pthread mutexes available.]) + HAVE_PTHREAD=1 + ], +- [AC_MSG_WARN([Pthread library not found! Clients will not be thread safe...])]) ++ [AC_MSG_WARN([Pthread mutex support not found! Clients will not be thread safe...])]) ++LIBS=$SAVE_LIBS ++AM_CONDITIONAL([HAVE_PTHREAD], [test x"$HAVE_PTHREAD" != "x"]) + + +-AM_CONDITIONAL([HAVE_PTHREAD], [test x"$HAVE_PTHREAD" != "x"]) ++SAVE_LIBS=$LIBS ++LIBS= ++AC_LINK_IFELSE( ++ [AC_LANG_PROGRAM([[#include ]], ++ [[static pthread_key_t k; ++ static pthread_once_t f = PTHREAD_ONCE_INIT; ++ pthread_once(&f, NULL); ++ pthread_key_create(&k, NULL); ++ ]])], ++ [AC_DEFINE([HAVE_PTHREAD_EXT], [1], [Extended pthread functionality is available.]) ++ HAVE_PTHREAD_EXT=1 ++ ], ++ [AC_MSG_WARN([Extended pthread functionality is not available. Lock-free client feature will not be built.])]) ++LIBS=$SAVE_LIBS ++AM_CONDITIONAL([BUILD_LOCKFREE_CLIENT], [test x"$HAVE_PTHREAD_EXT" != "x"]) ++ + + # Check library for the timer_create function + SAVE_LIBS=$LIBS +diff --git a/src/man/Makefile.am b/src/man/Makefile.am +index 93dd14819..063ff1bf0 100644 +--- a/src/man/Makefile.am ++++ b/src/man/Makefile.am +@@ -46,9 +46,12 @@ endif + if BUILD_KCM_RENEWAL + KCM_RENEWAL_CONDS = ;enable_kcm_renewal + endif ++if BUILD_LOCKFREE_CLIENT ++LOCKFREE_CLIENT_CONDS = ;enable_lockfree_support ++endif + + +-CONDS = with_false$(SUDO_CONDS)$(AUTOFS_CONDS)$(SSH_CONDS)$(PAC_RESPONDER_CONDS)$(IFP_CONDS)$(GPO_CONDS)$(SYSTEMD_CONDS)$(FILES_CONDS)$(KCM_CONDS)$(STAP_CONDS)$(KCM_RENEWAL_CONDS) ++CONDS = with_false$(SUDO_CONDS)$(AUTOFS_CONDS)$(SSH_CONDS)$(PAC_RESPONDER_CONDS)$(IFP_CONDS)$(GPO_CONDS)$(SYSTEMD_CONDS)$(FILES_CONDS)$(KCM_CONDS)$(STAP_CONDS)$(KCM_RENEWAL_CONDS)$(LOCKFREE_CLIENT_CONDS) + + + #Special Rules: +diff --git a/src/man/sssd.8.xml b/src/man/sssd.8.xml +index df07b7f29..5f507c631 100644 +--- a/src/man/sssd.8.xml ++++ b/src/man/sssd.8.xml +@@ -240,7 +240,7 @@ + If the environment variable SSS_NSS_USE_MEMCACHE is set to "NO", + client applications will not use the fast in-memory cache. + +- ++ + If the environment variable SSS_LOCKFREE is set to "NO", requests + from multiple threads of a single application will be serialized. + +diff --git a/src/sss_client/common.c b/src/sss_client/common.c +index 29c751a50..d762dff49 100644 +--- a/src/sss_client/common.c ++++ b/src/sss_client/common.c +@@ -35,7 +35,6 @@ + #include + #include + #include +-#include + #include + #include + #include +@@ -62,8 +61,15 @@ + + /* common functions */ + ++#ifdef HAVE_PTHREAD_EXT ++static pthread_key_t sss_sd_key; ++static pthread_once_t sss_sd_key_initialized = PTHREAD_ONCE_INIT; + static __thread int sss_cli_sd = -1; /* the sss client socket descriptor */ + static __thread struct stat sss_cli_sb; /* the sss client stat buffer */ ++#else ++static int sss_cli_sd = -1; /* the sss client socket descriptor */ ++static struct stat sss_cli_sb; /* the sss client stat buffer */ ++#endif + + #if HAVE_FUNCTION_ATTRIBUTE_DESTRUCTOR + __attribute__((destructor)) +@@ -76,6 +82,18 @@ void sss_cli_close_socket(void) + } + } + ++#ifdef HAVE_PTHREAD_EXT ++static void sss_at_thread_exit(void *v) ++{ ++ sss_cli_close_socket(); ++} ++ ++static void init_sd_key(void) ++{ ++ pthread_key_create(&sss_sd_key, sss_at_thread_exit); ++} ++#endif ++ + /* Requests: + * + * byte 0-3: 32bit unsigned with length (the complete packet length: 0 to X) +@@ -553,6 +571,16 @@ static int sss_cli_open_socket(int *errnop, const char *socket_name, int timeout + return -1; + } + ++#ifdef HAVE_PTHREAD_EXT ++ pthread_once(&sss_sd_key_initialized, init_sd_key); /* once for all threads */ ++ ++ /* It actually doesn't matter what value to set for a key. ++ * The only important thing: key must be non-NULL to ensure ++ * destructor is executed at thread exit. ++ */ ++ pthread_setspecific(sss_sd_key, &sss_cli_sd); ++#endif ++ + /* set as non-blocking, close on exec, and make sure standard + * descriptors are not used */ + sd = make_safe_fd(sd); +@@ -1129,41 +1157,38 @@ errno_t sss_strnlen(const char *str, size_t maxlen, size_t *len) + } + + #if HAVE_PTHREAD +-bool sss_is_lockfree_mode(void) ++ ++#ifdef HAVE_PTHREAD_EXT ++static bool sss_lock_free = true; ++static pthread_once_t sss_lock_mode_initialized = PTHREAD_ONCE_INIT; ++ ++static void init_lock_mode(void) + { +- const char *env = NULL; +- enum { +- MODE_UNDEF, +- MODE_LOCKING, +- MODE_LOCKFREE +- }; +- static atomic_int mode = MODE_UNDEF; +- +- if (mode == MODE_UNDEF) { +- env = getenv("SSS_LOCKFREE"); +- if ((env != NULL) && (strcasecmp(env, "NO") == 0)) { +- mode = MODE_LOCKING; +- } else { +- mode = MODE_LOCKFREE; +- } ++ const char *env = getenv("SSS_LOCKFREE"); ++ ++ if ((env != NULL) && (strcasecmp(env, "NO") == 0)) { ++ sss_lock_free = false; + } ++} + +- return (mode == MODE_LOCKFREE); ++bool sss_is_lockfree_mode(void) ++{ ++ pthread_once(&sss_lock_mode_initialized, init_lock_mode); ++ return sss_lock_free; + } ++#endif + + struct sss_mutex sss_nss_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER }; +- + static struct sss_mutex sss_pam_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER }; +- +-static struct sss_mutex sss_nss_mc_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER }; +- + static struct sss_mutex sss_pac_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER }; + + static void sss_mt_lock(struct sss_mutex *m) + { ++#ifdef HAVE_PTHREAD_EXT + if (sss_is_lockfree_mode()) { + return; + } ++#endif + + pthread_mutex_lock(&m->mtx); + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &m->old_cancel_state); +@@ -1171,9 +1196,11 @@ static void sss_mt_lock(struct sss_mutex *m) + + static void sss_mt_unlock(struct sss_mutex *m) + { ++#ifdef HAVE_PTHREAD_EXT + if (sss_is_lockfree_mode()) { + return; + } ++#endif + + pthread_setcancelstate(m->old_cancel_state, NULL); + pthread_mutex_unlock(&m->mtx); +@@ -1189,7 +1216,7 @@ void sss_nss_unlock(void) + sss_mt_unlock(&sss_nss_mtx); + } + +-/* NSS mutex wrappers */ ++/* PAM mutex wrappers */ + void sss_pam_lock(void) + { + sss_mt_lock(&sss_pam_mtx); +@@ -1199,16 +1226,6 @@ void sss_pam_unlock(void) + sss_mt_unlock(&sss_pam_mtx); + } + +-/* NSS mutex wrappers */ +-void sss_nss_mc_lock(void) +-{ +- sss_mt_lock(&sss_nss_mc_mtx); +-} +-void sss_nss_mc_unlock(void) +-{ +- sss_mt_unlock(&sss_nss_mc_mtx); +-} +- + /* PAC mutex wrappers */ + void sss_pac_lock(void) + { +diff --git a/src/sss_client/idmap/common_ex.c b/src/sss_client/idmap/common_ex.c +index 4f454cd63..8c4894fd9 100644 +--- a/src/sss_client/idmap/common_ex.c ++++ b/src/sss_client/idmap/common_ex.c +@@ -28,7 +28,9 @@ + #include "common_private.h" + + extern struct sss_mutex sss_nss_mtx; ++#ifdef HAVE_PTHREAD_EXT + bool sss_is_lockfree_mode(void); ++#endif + + #define SEC_FROM_MSEC(ms) ((ms) / 1000) + #define NSEC_FROM_MSEC(ms) (((ms) % 1000) * 1000 * 1000) +@@ -51,9 +53,11 @@ static int sss_mt_timedlock(struct sss_mutex *m, const struct timespec *endtime) + { + int ret; + ++#ifdef HAVE_PTHREAD_EXT + if (sss_is_lockfree_mode()) { + return 0; + } ++#endif + + ret = pthread_mutex_timedlock(&m->mtx, endtime); + if (ret != 0) { +-- +2.37.1 + diff --git a/sssd.spec b/sssd.spec index be99c00..b315d03 100644 --- a/sssd.spec +++ b/sssd.spec @@ -27,7 +27,7 @@ Name: sssd Version: 2.7.3 -Release: 3%{?dist} +Release: 4%{?dist} Summary: System Security Services Daemon License: GPLv3+ URL: https://github.com/SSSD/sssd/ @@ -43,6 +43,7 @@ Patch0006: 0006-CLIENT-MC-pointer-to-the-context-mutex-shouldn-t-be-.patch Patch0007: 0007-SSSCTL-Allow-analyzer-to-work-without-SSSD-setup.patch Patch0008: 0008-RESPONDER-Fix-client-ID-tracking.patch Patch0009: 0009-Analyzer-support-parallel-requests-parsing.patch +Patch0010: 0010-CLIENT-fix-client-fd-leak.patch ### Dependencies ### @@ -1067,6 +1068,9 @@ fi %systemd_postun_with_restart sssd.service %changelog +* Fri Aug 26 2022 Alexey Tikhonov - 2.7.3-4 +- Related: rhbz#1978119 - [Improvement] avoid interlocking among threads that use `libsss_nss_idmap` API (or other sss_client libs) + * Tue Aug 23 2022 Alexey Tikhonov - 2.7.3-3 - Resolves: rhbz#2116389 - rpc.gssd crash when access a same file on krb5 nfs mount with multiple uids simultaneously since sssd-2.7.3-2.el9 - Resolves: rhbz#2119373 - sssctl analyze --logdir option requires sssd to be configured