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