From 3caafdd88fa321d9daec98398a072124f09fe484 Mon Sep 17 00:00:00 2001 From: eabdullin Date: Wed, 21 Feb 2024 13:14:12 +0300 Subject: [PATCH] - nsswitch: add test for pthread_key_delete missuse (bug 15464) --- .../memory-corruption-since-samba-4-18.patch | 613 ++++++++++++++++++ SPECS/samba.spec | 8 +- 2 files changed, 620 insertions(+), 1 deletion(-) create mode 100644 SOURCES/memory-corruption-since-samba-4-18.patch diff --git a/SOURCES/memory-corruption-since-samba-4-18.patch b/SOURCES/memory-corruption-since-samba-4-18.patch new file mode 100644 index 0000000..f441e9d --- /dev/null +++ b/SOURCES/memory-corruption-since-samba-4-18.patch @@ -0,0 +1,613 @@ +From ced40c5a805dcfb06d5f3d68aa45a0aaa44bfdca Mon Sep 17 00:00:00 2001 +From: Stefan Metzmacher +Date: Fri, 8 Sep 2023 13:57:26 +0200 +Subject: [PATCH 1/5] nsswitch: add test for pthread_key_delete missuse (bug + 15464) + +This is based on https://bugzilla.samba.org/attachment.cgi?id=18081 +written by Krzysztof Piotr Oledzki + +BUG: https://bugzilla.samba.org/show_bug.cgi?id=15464 + +Signed-off-by: Stefan Metzmacher +Reviewed-by: Jeremy Allison +(cherry picked from commit 62af25d44e542548d8cdecb061a6001e0071ee76) +--- + nsswitch/b15464-testcase.c | 77 +++++++++++++++++++++++++++ + nsswitch/wscript_build | 5 ++ + selftest/knownfail.d/b15464_testcase | 1 + + source3/selftest/tests.py | 6 +++ + testprogs/blackbox/b15464-testcase.sh | 21 ++++++++ + 5 files changed, 110 insertions(+) + create mode 100644 nsswitch/b15464-testcase.c + create mode 100644 selftest/knownfail.d/b15464_testcase + create mode 100755 testprogs/blackbox/b15464-testcase.sh + +diff --git a/nsswitch/b15464-testcase.c b/nsswitch/b15464-testcase.c +new file mode 100644 +index 000000000000..decb474a81ee +--- /dev/null ++++ b/nsswitch/b15464-testcase.c +@@ -0,0 +1,77 @@ ++#include "replace.h" ++#include "system/wait.h" ++#include "system/threads.h" ++#include ++ ++int main(int argc, const char *argv[]) ++{ ++ pid_t pid; ++ int wstatus; ++ pthread_key_t k1; ++ pthread_key_t k2; ++ pthread_key_t k3; ++ char *val = NULL; ++ const char *nss_winbind = (argc >= 2 ? argv[1] : "bin/plugins/libnss_winbind.so.2"); ++ void *nss_winbind_handle = NULL; ++ union { ++ int (*fn)(void); ++ void *symbol; ++ } nss_winbind_endpwent = { .symbol = NULL, }; ++ ++ /* ++ * load and invoke something simple like ++ * _nss_winbind_endpwent in order to ++ * get the libnss_winbind internal going ++ */ ++ nss_winbind_handle = dlopen(nss_winbind, RTLD_NOW); ++ printf("%d: nss_winbind[%s] nss_winbind_handle[%p]\n", ++ getpid(), nss_winbind, nss_winbind_handle); ++ assert(nss_winbind_handle != NULL); ++ ++ nss_winbind_endpwent.symbol = dlsym(nss_winbind_handle, ++ "_nss_winbind_endpwent"); ++ printf("%d: nss_winbind_handle[%p] _nss_winbind_endpwent[%p]\n", ++ getpid(), nss_winbind_handle, nss_winbind_endpwent.symbol); ++ assert(nss_winbind_endpwent.symbol != NULL); ++ (void)nss_winbind_endpwent.fn(); ++ ++ val = malloc(1); ++ assert(val != NULL); ++ ++ pthread_key_create(&k1, NULL); ++ pthread_setspecific(k1, val); ++ printf("%d: k1=%d\n", getpid(), k1); ++ ++ pid = fork(); ++ if (pid) { ++ free(val); ++ wait(&wstatus); ++ return WEXITSTATUS(wstatus); ++ } ++ ++ pthread_key_create(&k2, NULL); ++ pthread_setspecific(k2, val); ++ ++ printf("%d: Hello after fork, k1=%d, k2=%d\n", getpid(), k1, k2); ++ ++ pid = fork(); ++ ++ if (pid) { ++ free(val); ++ wait(&wstatus); ++ return WEXITSTATUS(wstatus); ++ } ++ ++ pthread_key_create(&k3, NULL); ++ pthread_setspecific(k3, val); ++ ++ printf("%d: Hello after fork2, k1=%d, k2=%d, k3=%d\n", getpid(), k1, k2, k3); ++ ++ if (k1 == k2 || k2 == k3) { ++ printf("%d: FAIL inconsistent keys\n", getpid()); ++ return 1; ++ } ++ ++ printf("%d: OK consistent keys\n", getpid()); ++ return 0; ++} +diff --git a/nsswitch/wscript_build b/nsswitch/wscript_build +index 3247b6c2b7c3..4e62bb4c9461 100644 +--- a/nsswitch/wscript_build ++++ b/nsswitch/wscript_build +@@ -15,6 +15,11 @@ if bld.CONFIG_SET('HAVE_PTHREAD'): + deps='wbclient pthread', + for_selftest=True + ) ++ bld.SAMBA_BINARY('b15464-testcase', ++ source='b15464-testcase.c', ++ deps='replace pthread dl', ++ for_selftest=True ++ ) + + # The nss_wrapper code relies strictly on the linux implementation and + # name, so compile but do not install a copy under this name. +diff --git a/selftest/knownfail.d/b15464_testcase b/selftest/knownfail.d/b15464_testcase +new file mode 100644 +index 000000000000..94dd7db7c2a5 +--- /dev/null ++++ b/selftest/knownfail.d/b15464_testcase +@@ -0,0 +1 @@ ++^b15464_testcase.run.b15464-testcase +diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py +index 0c834ed48b5e..ea17ead3eda7 100755 +--- a/source3/selftest/tests.py ++++ b/source3/selftest/tests.py +@@ -67,6 +67,8 @@ except KeyError: + samba4bindir = bindir() + config_h = os.path.join(samba4bindir, "default/include/config.h") + ++bbdir = os.path.join(srcdir(), "testprogs/blackbox") ++ + # check available features + config_hash = dict() + f = open(config_h, 'r') +@@ -936,6 +938,10 @@ if with_pthreadpool: + [os.path.join(samba3srcdir, + "script/tests/test_libwbclient_threads.sh"), + "$DOMAIN", "$DC_USERNAME"]) ++ plantestsuite("b15464_testcase", "none", ++ [os.path.join(bbdir, "b15464-testcase.sh"), ++ binpath("b15464-testcase"), ++ binpath("plugins/libnss_winbind.so.2")]) + + plantestsuite("samba3.test_nfs4_acl", "none", + [os.path.join(bindir(), "test_nfs4_acls"), +diff --git a/testprogs/blackbox/b15464-testcase.sh b/testprogs/blackbox/b15464-testcase.sh +new file mode 100755 +index 000000000000..b0c88260d4cc +--- /dev/null ++++ b/testprogs/blackbox/b15464-testcase.sh +@@ -0,0 +1,21 @@ ++#!/bin/sh ++# Blackbox wrapper for bug 15464 ++# Copyright (C) 2023 Stefan Metzmacher ++ ++if [ $# -lt 2 ]; then ++ cat < +Date: Thu, 7 Sep 2023 16:02:32 +0200 +Subject: [PATCH 2/5] nsswitch/wb_common.c: fix build without HAVE_PTHREAD + +BUG: https://bugzilla.samba.org/show_bug.cgi?id=15464 + +Signed-off-by: Stefan Metzmacher +Reviewed-by: Jeremy Allison +(cherry picked from commit 4faf806412c4408db25448b1f67c09359ec2f81f) +--- + nsswitch/wb_common.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c +index d569e761ebe4..c382a44c1209 100644 +--- a/nsswitch/wb_common.c ++++ b/nsswitch/wb_common.c +@@ -104,7 +104,6 @@ static void wb_thread_ctx_initialize(void) + wb_thread_ctx_destructor); + assert(ret == 0); + } +-#endif + + static struct winbindd_context *get_wb_thread_ctx(void) + { +@@ -139,6 +138,7 @@ static struct winbindd_context *get_wb_thread_ctx(void) + } + return ctx; + } ++#endif /* HAVE_PTHREAD */ + + static struct winbindd_context *get_wb_global_ctx(void) + { +-- +2.34.1 + + +From d1f43cd4cc6aeb2ac9fcaee9aa512012ca92ecb3 Mon Sep 17 00:00:00 2001 +From: Stefan Metzmacher +Date: Fri, 8 Sep 2023 09:53:42 +0200 +Subject: [PATCH 3/5] nsswitch/wb_common.c: winbind_destructor can always use + get_wb_global_ctx() + +The HAVE_PTHREAD logic inside of get_wb_global_ctx() will do all +required magic. + +BUG: https://bugzilla.samba.org/show_bug.cgi?id=15464 + +Signed-off-by: Stefan Metzmacher +Reviewed-by: Jeremy Allison +(cherry picked from commit 836823e5047d0eb18e66707386ba03b812adfaf8) +--- + nsswitch/wb_common.c | 6 +----- + 1 file changed, 1 insertion(+), 5 deletions(-) + +diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c +index c382a44c1209..d56e48d9bdb8 100644 +--- a/nsswitch/wb_common.c ++++ b/nsswitch/wb_common.c +@@ -246,14 +246,10 @@ static void winbind_destructor(void) + return; + } + +-#ifdef HAVE_PTHREAD_H +- ctx = (struct winbindd_context *)pthread_getspecific(wb_global_ctx.key); ++ ctx = get_wb_global_ctx(); + if (ctx == NULL) { + return; + } +-#else +- ctx = get_wb_global_ctx(); +-#endif + + winbind_close_sock(ctx); + } +-- +2.34.1 + + +From 6e29ea5b9efe5cf166cc9d633c1dc4eb8f192736 Mon Sep 17 00:00:00 2001 +From: Stefan Metzmacher +Date: Fri, 8 Sep 2023 09:56:47 +0200 +Subject: [PATCH 4/5] nsswitch/wb_common.c: don't operate on a stale + wb_global_ctx.key + +If nss_winbind is loaded into a process that uses fork multiple times +without any further calls into nss_winbind, wb_atfork_child handler +was using a wb_global_ctx.key that was no longer registered in the +pthread library, so we operated on a slot that was potentially +reused by other libraries or the main application. Which is likely +to cause memory corruption. + +So we better don't call pthread_key_delete() in wb_atfork_child(). + +BUG: https://bugzilla.samba.org/show_bug.cgi?id=15464 + +Reported-by: Krzysztof Piotr Oledzki +Tested-by: Krzysztof Piotr Oledzki +Signed-off-by: Stefan Metzmacher +Reviewed-by: Jeremy Allison +(cherry picked from commit 91b30a7261e6455d3a4f31728c23e4849e3945b9) +--- + nsswitch/wb_common.c | 5 ----- + selftest/knownfail.d/b15464_testcase | 1 - + 2 files changed, 6 deletions(-) + delete mode 100644 selftest/knownfail.d/b15464_testcase + +diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c +index d56e48d9bdb8..38f9f334016b 100644 +--- a/nsswitch/wb_common.c ++++ b/nsswitch/wb_common.c +@@ -76,11 +76,6 @@ static void wb_atfork_child(void) + + winbind_close_sock(ctx); + free(ctx); +- +- ret = pthread_key_delete(wb_global_ctx.key); +- assert(ret == 0); +- +- wb_global_ctx.control = (pthread_once_t)PTHREAD_ONCE_INIT; + } + + static void wb_thread_ctx_destructor(void *p) +diff --git a/selftest/knownfail.d/b15464_testcase b/selftest/knownfail.d/b15464_testcase +deleted file mode 100644 +index 94dd7db7c2a5..000000000000 +--- a/selftest/knownfail.d/b15464_testcase ++++ /dev/null +@@ -1 +0,0 @@ +-^b15464_testcase.run.b15464-testcase +-- +2.34.1 + + +From 61ca2c66e0a3c837f2c542b8d9321a8d8cd03382 Mon Sep 17 00:00:00 2001 +From: Stefan Metzmacher +Date: Thu, 7 Sep 2023 15:59:59 +0200 +Subject: [PATCH 5/5] nsswitch/wb_common.c: fix socket fd and memory leaks of + global state + +When we are called in wb_atfork_child() or winbind_destructor(), +wb_thread_ctx_destructor() is not called for the global state +of the current nor any other thread, which means we would +leak the related memory and socket fds. + +Now we maintain a global list protected by a global mutex. +We traverse the list and close all socket fds, which are no +longer used (winbind_destructor) or no longer valid in the +current process (wb_atfork_child), in addition we 'autofree' +the ones, which are only visible internally as global (per thread) +context. + +BUG: https://bugzilla.samba.org/show_bug.cgi?id=15464 + +Tested-by: Krzysztof Piotr Oledzki +Signed-off-by: Stefan Metzmacher +Reviewed-by: Jeremy Allison + +Autobuild-User(master): Stefan Metzmacher +Autobuild-Date(master): Thu Sep 14 18:53:07 UTC 2023 on atb-devel-224 + +(cherry picked from commit 4af3faace481d23869b64485b791bdd43d8972c5) +--- + nsswitch/wb_common.c | 143 ++++++++++++++++++++++++++++++++++--------- + 1 file changed, 113 insertions(+), 30 deletions(-) + +diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c +index 38f9f334016b..b7f84435a4ee 100644 +--- a/nsswitch/wb_common.c ++++ b/nsswitch/wb_common.c +@@ -26,6 +26,7 @@ + #include "replace.h" + #include "system/select.h" + #include "winbind_client.h" ++#include "lib/util/dlinklist.h" + #include + + #ifdef HAVE_PTHREAD_H +@@ -37,67 +38,112 @@ static __thread char client_name[32]; + /* Global context */ + + struct winbindd_context { ++ struct winbindd_context *prev, *next; + int winbindd_fd; /* winbind file descriptor */ + bool is_privileged; /* using the privileged socket? */ + pid_t our_pid; /* calling process pid */ ++ bool autofree; /* this is a thread global context */ + }; + + static struct wb_global_ctx { +- bool initialized; + #ifdef HAVE_PTHREAD + pthread_once_t control; + pthread_key_t key; ++ bool key_initialized; ++#ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP ++#define WB_GLOBAL_MUTEX_INITIALIZER PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP + #else +- bool dummy; ++#define WB_GLOBAL_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER + #endif ++#define WB_GLOBAL_LIST_LOCK do { \ ++ int __pret = pthread_mutex_lock(&wb_global_ctx.list_mutex); \ ++ assert(__pret == 0); \ ++} while(0) ++#define WB_GLOBAL_LIST_UNLOCK do { \ ++ int __pret = pthread_mutex_unlock(&wb_global_ctx.list_mutex); \ ++ assert(__pret == 0); \ ++} while(0) ++ pthread_mutex_t list_mutex; ++#else /* => not HAVE_PTHREAD */ ++#define WB_GLOBAL_LIST_LOCK do { } while(0) ++#define WB_GLOBAL_LIST_UNLOCK do { } while(0) ++#endif /* not HAVE_PTHREAD */ ++ struct winbindd_context *list; + } wb_global_ctx = { + #ifdef HAVE_PTHREAD + .control = PTHREAD_ONCE_INIT, ++ .list_mutex = WB_GLOBAL_MUTEX_INITIALIZER, + #endif ++ .list = NULL, + }; + + static void winbind_close_sock(struct winbindd_context *ctx); ++static void winbind_ctx_free_locked(struct winbindd_context *ctx); ++static void winbind_cleanup_list(void); + + #ifdef HAVE_PTHREAD + static void wb_thread_ctx_initialize(void); + ++static void wb_atfork_prepare(void) ++{ ++ WB_GLOBAL_LIST_LOCK; ++} ++ ++static void wb_atfork_parent(void) ++{ ++ WB_GLOBAL_LIST_UNLOCK; ++} ++ + static void wb_atfork_child(void) + { +- struct winbindd_context *ctx = NULL; +- int ret; ++ wb_global_ctx.list_mutex = (pthread_mutex_t)WB_GLOBAL_MUTEX_INITIALIZER; + +- ctx = (struct winbindd_context *)pthread_getspecific(wb_global_ctx.key); +- if (ctx == NULL) { +- return; +- } ++ if (wb_global_ctx.key_initialized) { ++ int ret; + +- ret = pthread_setspecific(wb_global_ctx.key, NULL); +- assert(ret == 0); ++ /* ++ * After a fork the child still believes ++ * it is the same thread as in the parent. ++ * So pthread_getspecific() would return the ++ * value of the thread that called fork(). ++ * ++ * But we don't want that behavior, so ++ * we just clear the reference and let ++ * winbind_cleanup_list() below 'autofree' ++ * the parent threads global context. ++ */ ++ ret = pthread_setspecific(wb_global_ctx.key, NULL); ++ assert(ret == 0); ++ } + +- winbind_close_sock(ctx); +- free(ctx); ++ /* ++ * But we need to close/cleanup the global state ++ * of the parents threads. ++ */ ++ winbind_cleanup_list(); + } + + static void wb_thread_ctx_destructor(void *p) + { + struct winbindd_context *ctx = (struct winbindd_context *)p; + +- winbind_close_sock(ctx); +- free(ctx); ++ winbindd_ctx_free(ctx); + } + + static void wb_thread_ctx_initialize(void) + { + int ret; + +- ret = pthread_atfork(NULL, +- NULL, ++ ret = pthread_atfork(wb_atfork_prepare, ++ wb_atfork_parent, + wb_atfork_child); + assert(ret == 0); + + ret = pthread_key_create(&wb_global_ctx.key, + wb_thread_ctx_destructor); + assert(ret == 0); ++ ++ wb_global_ctx.key_initialized = true; + } + + static struct winbindd_context *get_wb_thread_ctx(void) +@@ -123,9 +169,14 @@ static struct winbindd_context *get_wb_thread_ctx(void) + *ctx = (struct winbindd_context) { + .winbindd_fd = -1, + .is_privileged = false, +- .our_pid = 0 ++ .our_pid = 0, ++ .autofree = true, + }; + ++ WB_GLOBAL_LIST_LOCK; ++ DLIST_ADD_END(wb_global_ctx.list, ctx); ++ WB_GLOBAL_LIST_UNLOCK; ++ + ret = pthread_setspecific(wb_global_ctx.key, ctx); + if (ret != 0) { + free(ctx); +@@ -142,7 +193,8 @@ static struct winbindd_context *get_wb_global_ctx(void) + static struct winbindd_context _ctx = { + .winbindd_fd = -1, + .is_privileged = false, +- .our_pid = 0 ++ .our_pid = 0, ++ .autofree = false, + }; + #endif + +@@ -150,9 +202,11 @@ static struct winbindd_context *get_wb_global_ctx(void) + ctx = get_wb_thread_ctx(); + #else + ctx = &_ctx; ++ if (ctx->prev == NULL && ctx->next == NULL) { ++ DLIST_ADD_END(wb_global_ctx.list, ctx); ++ } + #endif + +- wb_global_ctx.initialized = true; + return ctx; + } + +@@ -226,6 +280,30 @@ static void winbind_close_sock(struct winbindd_context *ctx) + } + } + ++static void winbind_ctx_free_locked(struct winbindd_context *ctx) ++{ ++ winbind_close_sock(ctx); ++ DLIST_REMOVE(wb_global_ctx.list, ctx); ++ free(ctx); ++} ++ ++static void winbind_cleanup_list(void) ++{ ++ struct winbindd_context *ctx = NULL, *next = NULL; ++ ++ WB_GLOBAL_LIST_LOCK; ++ for (ctx = wb_global_ctx.list; ctx != NULL; ctx = next) { ++ next = ctx->next; ++ ++ if (ctx->autofree) { ++ winbind_ctx_free_locked(ctx); ++ } else { ++ winbind_close_sock(ctx); ++ } ++ } ++ WB_GLOBAL_LIST_UNLOCK; ++} ++ + /* Destructor for global context to ensure fd is closed */ + + #ifdef HAVE_DESTRUCTOR_ATTRIBUTE +@@ -235,18 +313,18 @@ __attribute__((destructor)) + #endif + static void winbind_destructor(void) + { +- struct winbindd_context *ctx; +- +- if (!wb_global_ctx.initialized) { +- return; ++#ifdef HAVE_PTHREAD ++ if (wb_global_ctx.key_initialized) { ++ int ret; ++ ret = pthread_key_delete(wb_global_ctx.key); ++ assert(ret == 0); ++ wb_global_ctx.key_initialized = false; + } + +- ctx = get_wb_global_ctx(); +- if (ctx == NULL) { +- return; +- } ++ wb_global_ctx.control = (pthread_once_t)PTHREAD_ONCE_INIT; ++#endif /* HAVE_PTHREAD */ + +- winbind_close_sock(ctx); ++ winbind_cleanup_list(); + } + + #define CONNECT_TIMEOUT 30 +@@ -928,11 +1006,16 @@ struct winbindd_context *winbindd_ctx_create(void) + + ctx->winbindd_fd = -1; + ++ WB_GLOBAL_LIST_LOCK; ++ DLIST_ADD_END(wb_global_ctx.list, ctx); ++ WB_GLOBAL_LIST_UNLOCK; ++ + return ctx; + } + + void winbindd_ctx_free(struct winbindd_context *ctx) + { +- winbind_close_sock(ctx); +- free(ctx); ++ WB_GLOBAL_LIST_LOCK; ++ winbind_ctx_free_locked(ctx); ++ WB_GLOBAL_LIST_UNLOCK; + } +-- +2.34.1 diff --git a/SPECS/samba.spec b/SPECS/samba.spec index 6e830c8..88f7227 100644 --- a/SPECS/samba.spec +++ b/SPECS/samba.spec @@ -138,7 +138,7 @@ %define samba_requires_eq() %(LC_ALL="C" echo '%*' | xargs -r rpm -q --qf 'Requires: %%{name} = %%{epoch}:%%{version}\\n' | sed -e 's/ (none):/ /' -e 's/ 0:/ /' | grep -v "is not") %global samba_version 4.18.6 -%global baserelease 2 +%global baserelease 3 # This should be rc1 or %%nil %global pre_release %nil @@ -241,6 +241,8 @@ Patch1: CVE-2023-3961-s3-smbd-Catch-any-incoming-pipe-path-that.patch Patch2: CVE-2023-4091-smbtorture-test-overwrite-dispositions-on.patch # https://attachments.samba.org/attachment.cgi?id=18136 Patch3: CVE-2023-42669-s4-rpc_server-Disable-rpcecho-server-by.patch +# https://attachments.samba.org/attachment.cgi?id=18104 +Patch4: memory-corruption-since-samba-4-18.patch Requires(pre): /usr/sbin/groupadd @@ -4335,6 +4337,10 @@ fi %endif %changelog +* Wed Feb 21 2024 Eduard Abdullin - 4.18.6-3.alma.1 +- Fix libnss_winbind causes memory corruption since samba-4.18, + impacts sendmail, zabbix, potentially more + * Thu Nov 23 2023 Eduard Abdullin - 4.18.6-2.alma.1 - CVE-2023-3961:s3:smbd: Catch any incoming pipe path that could exit socket_dir.