296 lines
8.8 KiB
Diff
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
|
|
|