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