Merge ~lvoytek/ubuntu/+source/sssd:fix-client-fd-leak-kinetic into ubuntu/+source/sssd:ubuntu/kinetic-devel

[Lena Voytek]

PPA: https://launchpad.net/~lvoytek/+archive/ubuntu/sssd-fix-client-fd-leak

autopkgtest results:
  sssd @ amd64:
    25.01.23 22:01:50 Log 🗒️ ✅ Triggers: sssd/2.7.3-2ubuntu3~ppa2
  sssd @ arm64:
    03.02.23 20:48:14 Log 🗒️ ✅ Triggers: sssd/2.7.3-2ubuntu3~ppa2
  sssd @ armhf:
    03.02.23 20:47:14 Log 🗒️ ✅ Triggers: sssd/2.7.3-2ubuntu3~ppa2
  sssd @ ppc64el:
    03.02.23 20:43:14 Log 🗒️ ✅ Triggers: sssd/2.7.3-2ubuntu3~ppa2
  sssd @ s390x:
    03.02.23 20:48:50 Log 🗒️ ✅ Triggers: sssd/2.7.3-2ubuntu3~ppa2

[Athos Ribeiro (athos-ribeiro)]

Thanks, Lena!

LGTM. I did add a couple nitpick comments inline below.

review: Needs Information

575d671... by Lena Voytek on 2023-02-08

changelog

3180ab1... by Lena Voytek on 2023-02-08

* d/p/fix-client-fd-leak.patch:
  - 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

[Lena Voytek (lvoytek)]

Thanks for the review Athos! Updated based on your comments

[Athos Ribeiro (athos-ribeiro)]

LGTM! Thanks :)

review: Approve

[git-ubuntu bot (git-ubuntu-bot)]

Approvers: lvoytek, athos-ribeiro
Uploaders: lvoytek, athos-ribeiro
MP auto-approved

review: Approve

[Lena Voytek (lvoytek)]

Uploaded, thanks!
dput ubuntu ../sssd_2.7.3-2ubuntu2.1_source.changes
D: Setting host argument.
Checking signature on .changes
gpg: ../sssd_2.7.3-2ubuntu2.1_source.changes: Valid signature from 34B8AD7D9529E793
Checking signature on .dsc
gpg: ../sssd_2.7.3-2ubuntu2.1.dsc: Valid signature from 34B8AD7D9529E793
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading sssd_2.7.3-2ubuntu2.1.dsc: done.
  Uploading sssd_2.7.3-2ubuntu2.1.debian.tar.xz: done.
  Uploading sssd_2.7.3-2ubuntu2.1_source.buildinfo: done.
  Uploading sssd_2.7.3-2ubuntu2.1_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index e1685a4..45e547c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+sssd (2.7.3-2ubuntu2.1) kinetic; urgency=medium
+
+  * d/p/fix-client-fd-leak.patch (LP: #1996869):
+    - 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
+
+ -- Lena Voytek <[e-mail]>  Fri, 20 Jan 2023 11:03:59 -0700
+
 sssd (2.7.3-2ubuntu2) kinetic; urgency=medium
 
   * d/p/initialize-uid-gid-main-functions.patch: Initialize UID/GID
diff --git a/debian/patches/fix-client-fd-leak.patch b/debian/patches/fix-client-fd-leak.patch
new file mode 100644
index 0000000..1521eee
--- /dev/null
+++ b/debian/patches/fix-client-fd-leak.patch
@@ -0,0 +1,268 @@
+Description: Fix client fd leak
+ Avoid leaking old client sockets by closing them properly on exit.
+ Also note that Lock-free client support will only be built if libc provides
+ `pthread_key_create()` and `pthread_once()`. This is relevant in glibc
+ version 2.34+.
+Author: Alexey Tikhonov <[e-mail]>
+Origin: upstream, https://github.com/SSSD/sssd/commit/1a6f67c92399ff8e358a6c6cdda43fb2547a5fdb
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1996869
+Last-Update: 2023-01-25
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+--- a/configure.ac
++++ b/configure.ac
+@@ -51,18 +51,39 @@
+ 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
+--- a/src/man/Makefile.am
++++ b/src/man/Makefile.am
+@@ -46,9 +46,12 @@
+ 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:
+--- 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>
+--- 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 @@
+     }
+ }
+ 
++#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 @@
+         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 @@
+ }
+ 
+ #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_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 @@
+     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 @@
+     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)
+ {
+--- 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 @@
+ {
+     int ret;
+ 
++#ifdef HAVE_PTHREAD_EXT
+     if (sss_is_lockfree_mode()) {
+         return 0;
+     }
++#endif
+ 
+     ret = pthread_mutex_timedlock(&m->mtx, endtime);
+     if (ret != 0) {
diff --git a/debian/patches/series b/debian/patches/series
index b5cc56b..f47c99f 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -3,3 +3,4 @@ default-to-socket-activated-services.diff
 fix-shebang-on-sss_analyze.patch
 support-krb5-1.20.diff
 initialize-uid-gid-main-functions.patch
+fix-client-fd-leak.patch