From e0b142320342ef16260b6072f1c83d6fcf4142e6 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) (cherry picked from commit 89dc0ee157caa4617d32fd72287849296d7fe26d) --- 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);