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