From e0b142320342ef16260b6072f1c83d6fcf4142e6 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
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 <simo@redhat.com>
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
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 <pthread.h>
-__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);