diff --git a/Use-pthread-keys-for-thread-local-storage.patch b/Use-pthread-keys-for-thread-local-storage.patch new file mode 100644 index 0000000..d80db05 --- /dev/null +++ b/Use-pthread-keys-for-thread-local-storage.patch @@ -0,0 +1,149 @@ +From 89dc0ee157caa4617d32fd72287849296d7fe26d Mon Sep 17 00:00:00 2001 +From: Simo Sorce +Date: Thu, 20 Sep 2018 17:37:53 -0400 +Subject: [PATCH] Use pthread keys for thread local storage + +This interface is slower but also more portable, and more importantly +it provides a way to specify destructor that is called when a thread +is canceled so we stop leaking memory. + +Signed-off-by: Simo Sorce +Reviewed-by: Robbie Harwood +Merges: #233 +(cherry picked from commit 0faccc1441bc7a6b3e8bd806f22c8a961e5f586e) +--- + src/client/gpm_common.c | 2 ++ + src/client/gpm_display_status.c | 57 ++++++++++++++++++++++----------- + src/client/gssapi_gpm.h | 1 + + 3 files changed, 42 insertions(+), 18 deletions(-) + +diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c +index dd29519..c254280 100644 +--- a/src/client/gpm_common.c ++++ b/src/client/gpm_common.c +@@ -55,6 +55,8 @@ static void gpm_init_once(void) + gpm_global_ctx.next_xid = rand_r(&seedp); + + pthread_mutexattr_destroy(&attr); ++ ++ gpm_display_status_init_once(); + } + + static int get_pipe_name(char *name) +diff --git a/src/client/gpm_display_status.c b/src/client/gpm_display_status.c +index bbb546f..e3aa4ea 100644 +--- a/src/client/gpm_display_status.c ++++ b/src/client/gpm_display_status.c +@@ -1,27 +1,47 @@ + /* Copyright (C) 2011 the GSS-PROXY contributors, see COPYING for license */ + + #include "gssapi_gpm.h" ++#include + +-__thread gssx_status *tls_last_status = NULL; ++static pthread_key_t gpm_last_status; + +-/* Thread local storage for return status. +- * FIXME: it's not the most portable construct, so may need fixing in future */ ++static void gpm_destroy_last_status(void *arg) ++{ ++ gssx_status *status = (gssx_status *)arg; ++ xdr_free((xdrproc_t)xdr_gssx_status, (char *)status); ++ free(status); ++} ++ ++void gpm_display_status_init_once(void) ++{ ++ (void)pthread_key_create(&gpm_last_status, gpm_destroy_last_status); ++} ++ ++/* Portable thread local storage for return status. */ + void gpm_save_status(gssx_status *status) + { ++ gssx_status *last_status; + int ret; + +- if (tls_last_status) { +- xdr_free((xdrproc_t)xdr_gssx_status, (char *)tls_last_status); +- free(tls_last_status); ++ last_status = (gssx_status *)pthread_getspecific(gpm_last_status); ++ if (last_status != NULL) { ++ /* store NULL first so we do not risk a double free if we are ++ * racing on a pthread_cancel */ ++ pthread_setspecific(gpm_last_status, NULL); ++ gpm_destroy_last_status(last_status); + } + +- ret = gp_copy_gssx_status_alloc(status, &tls_last_status); +- if (ret) { +- /* make sure tls_last_status is zeored on error */ +- tls_last_status = NULL; ++ ret = gp_copy_gssx_status_alloc(status, &last_status); ++ if (ret == 0) { ++ pthread_setspecific(gpm_last_status, last_status); + } + } + ++gssx_status *gpm_get_saved_status(void) ++{ ++ return (gssx_status *)pthread_getspecific(gpm_last_status); ++} ++ + /* This funciton is used to record internal mech errors that are + * generated by the proxy client code */ + void gpm_save_internal_status(uint32_t err, char *err_str) +@@ -47,15 +67,16 @@ OM_uint32 gpm_display_status(OM_uint32 *minor_status, + OM_uint32 *message_context, + gss_buffer_t status_string) + { ++ gssx_status *last_status = gpm_get_saved_status(); + utf8string tmp; + int ret; + + switch(status_type) { + case GSS_C_GSS_CODE: +- if (tls_last_status && +- tls_last_status->major_status == status_value && +- tls_last_status->major_status_string.utf8string_len) { +- ret = gp_copy_utf8string(&tls_last_status->major_status_string, ++ if (last_status && ++ last_status->major_status == status_value && ++ last_status->major_status_string.utf8string_len) { ++ ret = gp_copy_utf8string(&last_status->major_status_string, + &tmp); + if (ret) { + *minor_status = ret; +@@ -70,9 +91,9 @@ OM_uint32 gpm_display_status(OM_uint32 *minor_status, + return GSS_S_UNAVAILABLE; + } + case GSS_C_MECH_CODE: +- if (tls_last_status && +- tls_last_status->minor_status == status_value && +- tls_last_status->minor_status_string.utf8string_len) { ++ if (last_status && ++ last_status->minor_status == status_value && ++ last_status->minor_status_string.utf8string_len) { + + if (*message_context) { + /* we do not support multiple messages for now */ +@@ -80,7 +101,7 @@ OM_uint32 gpm_display_status(OM_uint32 *minor_status, + return GSS_S_FAILURE; + } + +- ret = gp_copy_utf8string(&tls_last_status->minor_status_string, ++ ret = gp_copy_utf8string(&last_status->minor_status_string, + &tmp); + if (ret) { + *minor_status = ret; +diff --git a/src/client/gssapi_gpm.h b/src/client/gssapi_gpm.h +index 22beecf..61124e0 100644 +--- a/src/client/gssapi_gpm.h ++++ b/src/client/gssapi_gpm.h +@@ -23,6 +23,7 @@ OM_uint32 gpm_release_name(OM_uint32 *minor_status, + OM_uint32 gpm_release_buffer(OM_uint32 *minor_status, + gss_buffer_t buffer); + ++void gpm_display_status_init_once(void); + void gpm_save_status(gssx_status *status); + void gpm_save_internal_status(uint32_t err, char *err_str); + diff --git a/gssproxy.spec b/gssproxy.spec index b18097b..e16dc08 100644 --- a/gssproxy.spec +++ b/gssproxy.spec @@ -1,7 +1,7 @@ Name: gssproxy Version: 0.8.0 -Release: 6%{?dist} +Release: 7%{?dist} Summary: GSSAPI Proxy Group: System Environment/Libraries @@ -18,6 +18,7 @@ Patch0: Always-use-the-encype-we-selected.patch Patch1: Clarify-debug-and-debug_level-in-man-pages.patch Patch2: Always-choose-highest-requested-debug-level.patch Patch3: Don-t-leak-sock_ctx-if-verto_add_io-fails.patch +Patch4: Use-pthread-keys-for-thread-local-storage.patch ### Dependencies ### Requires: krb5-libs >= 1.12.0 @@ -111,6 +112,9 @@ mkdir -p %{buildroot}%{gpstatedir}/rcache %systemd_postun_with_restart gssproxy.service %changelog +* Thu Sep 20 2018 Robbie Harwood - 0.8.0-7 +- Use pthread keys for thread local storage + * Fri Aug 03 2018 Robbie Harwood - 0.8.0-6 - Don't leak sock_ctx if verto_add_io() fails