sssd/SOURCES/0010-CLIENT-fix-client-fd-l...

296 lines
8.8 KiB
Diff

From 1b2e4760c52b9abd0d9b9f35b47ed72e79922ccc Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikhono@redhat.com>
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 <jstephen@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
(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 <errno.h>]])
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.h>]],
[[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 <pthread.h>]],
+ [[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.
</para>
- <para>
+ <para condition="enable_lockfree_support">
If the environment variable SSS_LOCKFREE is set to "NO", requests
from multiple threads of a single application will be serialized.
</para>
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 <stdlib.h>
#include <stdbool.h>
#include <stdint.h>
-#include <stdatomic.h>
#include <string.h>
#include <fcntl.h>
#include <poll.h>
@@ -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