From 17dbcfaff5e7d917eb48579dc8e5f60c95a30981 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)
---
 proxy/src/client/gpm_common.c         |  2 +
 proxy/src/client/gpm_display_status.c | 57 ++++++++++++++++++---------
 proxy/src/client/gssapi_gpm.h         |  1 +
 3 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c
index 0d314fa..b482efb 100644
--- a/proxy/src/client/gpm_common.c
+++ b/proxy/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/proxy/src/client/gpm_display_status.c b/proxy/src/client/gpm_display_status.c
index bbb546f..e3aa4ea 100644
--- a/proxy/src/client/gpm_display_status.c
+++ b/proxy/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/proxy/src/client/gssapi_gpm.h b/proxy/src/client/gssapi_gpm.h
index 22beecf..61124e0 100644
--- a/proxy/src/client/gssapi_gpm.h
+++ b/proxy/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);