rpms/gssproxy
6
0
Fork 0
Code Issues Pull Requests Releases Activity

Avoid uninitialized free when allocating buffers

Browse Source
This commit is contained in:
Robbie Harwood 2019-05-01 16:52:16 +00:00
parent 2aa6bf716d
commit 5845ed62e4
11 changed files with 44 additions and 731 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
Add-a-safety-timeout-to-epoll.patch
View File

@ -1,47 +0,0 @@
From 616620ad1f8568437da8d9ec235ffa2f2ec9ca32 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Wed, 6 Mar 2019 10:36:11 -0500
Subject: [PATCH] Add a safety timeout to epoll
Add a safety timeout just in case something goes wrong with the use of
timerfd. This way the process should't be stuck forever.
Signed-off-by: Simo Sorce <simo@redhat.com>
[rharwood@redhat.com: remove outdated comment]
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Merges: #241
(cherry picked from commit d55be9fa2455fe52b6eb904ad427f22141ab3f26)
---
src/client/gpm_common.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c
index d491200..95a39ab 100644
--- a/src/client/gpm_common.c
+++ b/src/client/gpm_common.c
@@ -14,6 +14,7 @@
#define FRAGMENT_BIT (1 << 31)
#define RESPONSE_TIMEOUT 15
+#define SAFETY_TIMEOUT RESPONSE_TIMEOUT * 10 * 1000
#define MAX_TIMEOUT_RETRY 3
struct gpm_ctx {
@@ -291,7 +292,7 @@ static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags)
}
do {
- epoll_ret = epoll_wait(gpmctx->epollfd, events, 2, -1);
+ epoll_ret = epoll_wait(gpmctx->epollfd, events, 2, SAFETY_TIMEOUT);
} while (epoll_ret < 0 && errno == EINTR);
if (epoll_ret < 0) {
@@ -299,8 +300,6 @@ static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags)
ret = errno;
gpm_epoll_close(gpmctx);
} else if (epoll_ret == 0) {
- /* Shouldn't happen as timeout == -1; treat it like a timeout
- * occurred. */
ret = ETIMEDOUT;
gpm_epoll_close(gpmctx);
} else if (epoll_ret == 1 && events[0].data.fd == gpmctx->timerfd) {

107
Always-choose-highest-requested-debug-level.patch
View File

@ -1,107 +0,0 @@
From d284ec7dc9fe0a824b177873078aeb36a25b7878 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Wed, 11 Apr 2018 16:15:00 -0400
Subject: [PATCH] Always choose highest requested debug level
Allowing the CLI to lower the debug level specified in a config file
is dubious, and previously broken since we don't distinguish "default
value" from "explicitly requested value of 0" in popt. This resulted
in "Debug Enabled (level: 0)" even when the log level was not actually
0, which is confusing for users.
Remove the gp_debug_args() function since it is no longer used.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
Merges: #229
(cherry picked from commit 5a714768aec776dc875237dd729c85389932a688)
---
src/gp_debug.c | 34 ++++++++--------------------------
src/gp_debug.h | 3 +--
src/gssproxy.c | 2 +-
3 files changed, 10 insertions(+), 29 deletions(-)
diff --git a/src/gp_debug.c b/src/gp_debug.c
index 4a141fc..a0f51f0 100644
--- a/src/gp_debug.c
+++ b/src/gp_debug.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2011 the GSS-PROXY contributors, see COPYING for license */
+/* Copyright (C) 2011,2018 the GSS-PROXY contributors, see COPYING for license */
#include "config.h"
#include <stdbool.h>
@@ -7,35 +7,17 @@
#include "gp_log.h"
/* global debug switch */
-int gp_debug;
-
-int gp_debug_args(int level) {
- static int args_level = 0;
-
- if (level != 0) {
- args_level = level;
- }
- return args_level;
-}
+int gp_debug = 0;
void gp_debug_toggle(int level)
{
- static bool krb5_trace_set = false;
+ if (level <= gp_debug)
+ return;
- /* Command line and environment options override config file */
- gp_debug = gp_debug_args(0);
- if (gp_debug == 0) {
- gp_debug = level;
- }
- if (level >= 3) {
- if (!getenv("KRB5_TRACE")) {
- setenv("KRB5_TRACE", "/dev/stderr", 1);
- krb5_trace_set = true;
- }
- } else if (krb5_trace_set) {
- unsetenv("KRB5_TRACE");
- krb5_trace_set = false;
- }
+ if (level >= 3 && !getenv("KRB5_TRACE"))
+ setenv("KRB5_TRACE", "/dev/stderr", 1);
+
+ gp_debug = level;
GPDEBUG("Debug Enabled (level: %d)\n", level);
}
diff --git a/src/gp_debug.h b/src/gp_debug.h
index 1c2f8a3..4932bfd 100644
--- a/src/gp_debug.h
+++ b/src/gp_debug.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 2011 the GSS-PROXY contributors, see COPYING for license */
+/* Copyright (C) 2011,2018 the GSS-PROXY contributors, see COPYING for license */
#ifndef _GP_DEBUG_H_
#define _GP_DEBUG_H_
@@ -10,7 +10,6 @@
extern int gp_debug;
-int gp_debug_args(int level);
void gp_debug_toggle(int);
void gp_debug_printf(const char *format, ...);
void gp_debug_time_printf(const char *format, ...);
diff --git a/src/gssproxy.c b/src/gssproxy.c
index 6d36a5d..db6e89b 100644
--- a/src/gssproxy.c
+++ b/src/gssproxy.c
@@ -208,7 +208,7 @@ int main(int argc, const char *argv[])
if (opt_debug || opt_debug_level > 0) {
if (opt_debug_level == 0) opt_debug_level = 1;
- gp_debug_args(opt_debug_level);
+ gp_debug_toggle(opt_debug_level);
}
if (opt_daemon && opt_interactive) {

43
Always-use-the-encype-we-selected.patch
View File

@ -1,43 +0,0 @@
From 64bf7f099fe52a214794486d16e3383ff25e8682 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Tue, 27 Feb 2018 11:59:25 -0500
Subject: [PATCH] Always use the encype we selected
The enctype is selected from the keytab or from the fallback code.
Either way make sure to use the enctype stored in the key block.
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Merges: #226
(cherry picked from commit d73c96d658059ce64ecd41ff2924071d86f2b54f)
---
src/gp_export.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/gp_export.c b/src/gp_export.c
index c9f5fd4..5e8e160 100644
--- a/src/gp_export.c
+++ b/src/gp_export.c
@@ -168,11 +168,10 @@ uint32_t gp_init_creds_handle(uint32_t *min, const char *svc_name,
GP_CREDS_HANDLE_KEY_ENCTYPE, 0,
&handle->key);
if (ret == 0) {
- ret = krb5_c_make_random_key(handle->context,
- GP_CREDS_HANDLE_KEY_ENCTYPE,
+ ret = krb5_c_make_random_key(handle->context, handle->key->enctype,
handle->key);
GPDEBUG("Service: %s, Enckey: [ephemeral], Enctype: %d\n",
- svc_name, GP_CREDS_HANDLE_KEY_ENCTYPE);
+ svc_name, handle->key->enctype);
}
if (ret) {
ret_min = ret;
@@ -254,7 +253,7 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key,
memset(&enc_handle, '\0', sizeof(krb5_enc_data));
- enc_handle.enctype = GP_CREDS_HANDLE_KEY_ENCTYPE;
+ enc_handle.enctype = key->enctype;
enc_handle.ciphertext.data = in->octet_string_val;
enc_handle.ciphertext.length = in->octet_string_len;

39
Avoid-uninitialized-free-when-allocating-buffers.patch Normal file
View File

@ -0,0 +1,39 @@
From e19466d172e0fd6d86b98b1423e9d99e0be30313 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Wed, 1 May 2019 11:27:13 -0400
Subject: [PATCH] Avoid uninitialized free when allocating buffers
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
Resolves: #248
(cherry picked from commit eafa3c9272c95646400123f8e4d6fb50cf36d36c)
---
src/gp_export.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/gp_export.c b/src/gp_export.c
index dbfddeb..a5681c0 100644
--- a/src/gp_export.c
+++ b/src/gp_export.c
@@ -300,6 +300,7 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
&data_in,
&enc_handle);
if (ret) {
+ free(enc_handle.ciphertext.data);
ret = EINVAL;
goto done;
}
@@ -308,12 +309,12 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
enc_handle.ciphertext.data,
out);
if (ret) {
+ free(enc_handle.ciphertext.data);
goto done;
}
done:
free(padded);
- free(enc_handle.ciphertext.data);
return ret;
}

74
Clarify-debug-and-debug_level-in-man-pages.patch
View File

@ -1,74 +0,0 @@
From d71d354f1020a7deac57f26cc7c2cafb3fa675a3 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Wed, 11 Apr 2018 16:01:21 -0400
Subject: [PATCH] Clarify debug and debug_level in man pages
In particular, add debug_level to gssproxy(5) since it was previously
accepted but not documented.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
Merges: #229
(cherry picked from commit e0e96e46be03102903533a9816b4deefe1adfaf8)
---
man/gssproxy.8.xml | 24 +++++++++++++++++++++++-
man/gssproxy.conf.5.xml | 5 ++++-
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/man/gssproxy.8.xml b/man/gssproxy.8.xml
index 1df4b0d..21f7e6a 100644
--- a/man/gssproxy.8.xml
+++ b/man/gssproxy.8.xml
@@ -118,13 +118,35 @@
</para>
</listitem>
</varlistentry>
+
<varlistentry>
<term>
<option>-d</option>,<option>--debug</option>
</term>
<listitem>
<para>
- Turn on debugging.
+ Turn on debugging. This option is identical to
+ --debug-level=1.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>
+ <option>--debug-level=</option>
+ </term>
+ <listitem>
+ <para>
+ Turn on debugging at the specified level. 0
+ corresponds to no logging, while 1 turns on basic
+ debug logging. Level 2 increases verbosity, including
+ more detailed credential verification.
+ </para>
+ <para>
+ At level 3 and above, KRB5_TRACE output is logged. If
+ KRB5_TRACE was already set in the execution
+ environment, trace output is sent to its value
+ instead.
</para>
</listitem>
</varlistentry>
diff --git a/man/gssproxy.conf.5.xml b/man/gssproxy.conf.5.xml
index de846b4..21c9653 100644
--- a/man/gssproxy.conf.5.xml
+++ b/man/gssproxy.conf.5.xml
@@ -192,7 +192,10 @@
<varlistentry>
<term>debug (boolean)</term>
<listitem>
- <para>Enable debugging to syslog.</para>
+ <para>
+ Enable debugging to syslog. Setting to true is
+ identical to setting debug_level to 1.
+ </para>
<para>Default: debug = false</para>
</listitem>
</varlistentry>

174
Close-epoll-fd-within-the-lock.patch
View File

@ -1,174 +0,0 @@
From 9c0c6b8202ecd20dc25ea37bab7b7297cfca077f Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Wed, 6 Mar 2019 10:31:13 -0500
Subject: [PATCH] Close epoll fd within the lock
A race condition may happen where we close the epoll socket, after
another thread grabbed the lock and is using epoll itself.
On some kernels this may cause epoll to not fire any event leaving the
thread stuck forever.
Signed-off-by: Simo Sorce <simo@redhat.com>
[rharwood@redhat.com: cleanup commit message, adjusted function ordering]
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Merges: #241
(cherry picked from commit 0ccfd32f8ef16caf65698c5319dfa251d43433af)
---
src/client/gpm_common.c | 106 +++++++++++++++++++++-------------------
1 file changed, 56 insertions(+), 50 deletions(-)
diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c
index c254280..d491200 100644
--- a/src/client/gpm_common.c
+++ b/src/client/gpm_common.c
@@ -139,41 +139,14 @@ static void gpm_close_socket(struct gpm_ctx *gpmctx)
gpmctx->fd = -1;
}
-static int gpm_grab_sock(struct gpm_ctx *gpmctx)
+static void gpm_epoll_close(struct gpm_ctx *gpmctx)
{
- int ret;
- pid_t p;
- uid_t u;
- gid_t g;
-
- ret = pthread_mutex_lock(&gpmctx->lock);
- if (ret) {
- return ret;
+ if (gpmctx->epollfd < 0) {
+ return;
}
- /* Detect fork / setresuid and friends */
- p = getpid();
- u = geteuid();
- g = getegid();
-
- if (gpmctx->fd != -1 &&
- (p != gpmctx->pid || u != gpmctx->uid || g != gpmctx->gid)) {
- gpm_close_socket(gpmctx);
- }
-
- if (gpmctx->fd == -1) {
- ret = gpm_open_socket(gpmctx);
- }
-
- if (ret) {
- pthread_mutex_unlock(&gpmctx->lock);
- }
- return ret;
-}
-
-static int gpm_release_sock(struct gpm_ctx *gpmctx)
-{
- return pthread_mutex_unlock(&gpmctx->lock);
+ close(gpmctx->epollfd);
+ gpmctx->epollfd = -1;
}
static void gpm_timer_close(struct gpm_ctx *gpmctx)
@@ -186,6 +159,13 @@ static void gpm_timer_close(struct gpm_ctx *gpmctx)
gpmctx->timerfd = -1;
}
+static int gpm_release_sock(struct gpm_ctx *gpmctx)
+{
+ gpm_epoll_close(gpmctx);
+ gpm_timer_close(gpmctx);
+ return pthread_mutex_unlock(&gpmctx->lock);
+}
+
static int gpm_timer_setup(struct gpm_ctx *gpmctx, int timeout_seconds)
{
int ret;
@@ -216,16 +196,6 @@ static int gpm_timer_setup(struct gpm_ctx *gpmctx, int timeout_seconds)
return 0;
}
-static void gpm_epoll_close(struct gpm_ctx *gpmctx)
-{
- if (gpmctx->epollfd < 0) {
- return;
- }
-
- close(gpmctx->epollfd);
- gpmctx->epollfd = -1;
-}
-
static int gpm_epoll_setup(struct gpm_ctx *gpmctx)
{
struct epoll_event ev;
@@ -253,6 +223,50 @@ static int gpm_epoll_setup(struct gpm_ctx *gpmctx)
return ret;
}
+static int gpm_grab_sock(struct gpm_ctx *gpmctx)
+{
+ int ret;
+ pid_t p;
+ uid_t u;
+ gid_t g;
+
+ ret = pthread_mutex_lock(&gpmctx->lock);
+ if (ret) {
+ return ret;
+ }
+
+ /* Detect fork / setresuid and friends */
+ p = getpid();
+ u = geteuid();
+ g = getegid();
+
+ if (gpmctx->fd != -1 &&
+ (p != gpmctx->pid || u != gpmctx->uid || g != gpmctx->gid)) {
+ gpm_close_socket(gpmctx);
+ }
+
+ if (gpmctx->fd == -1) {
+ ret = gpm_open_socket(gpmctx);
+ if (ret) {
+ goto done;
+ }
+ }
+
+ /* setup timer */
+ ret = gpm_timer_setup(gpmctx, RESPONSE_TIMEOUT);
+ if (ret) {
+ goto done;
+ }
+ /* create epoll fd as well */
+ ret = gpm_epoll_setup(gpmctx);
+
+done:
+ if (ret) {
+ gpm_release_sock(gpmctx);
+ }
+ return ret;
+}
+
static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags)
{
int ret;
@@ -530,11 +544,6 @@ static int gpm_send_recv_loop(struct gpm_ctx *gpmctx, char *send_buffer,
int ret;
int retry_count;
- /* setup timer */
- ret = gpm_timer_setup(gpmctx, RESPONSE_TIMEOUT);
- if (ret)
- return ret;
-
for (retry_count = 0; retry_count < MAX_TIMEOUT_RETRY; retry_count++) {
/* send to proxy */
ret = gpm_send_buffer(gpmctx, send_buffer, send_length);
@@ -761,9 +770,6 @@ int gpm_make_call(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res)
}
done:
- gpm_timer_close(gpmctx);
- gpm_epoll_close(gpmctx);
-
if (sockgrab) {
gpm_release_sock(gpmctx);
}

23
Don-t-leak-sock_ctx-if-verto_add_io-fails.patch
View File

@ -1,23 +0,0 @@
From 322a7e578cc1f3b54bfb317dd57442231a8f7cf7 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Thu, 2 Aug 2018 16:02:50 -0400
Subject: [PATCH] Don't leak sock_ctx if verto_add_io() fails
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
(cherry picked from commit 459152be1e701af6aafdecffc1af21156b43bf78)
---
src/gssproxy.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/gssproxy.c b/src/gssproxy.c
index db6e89b..93c1c1e 100644
--- a/src/gssproxy.c
+++ b/src/gssproxy.c
@@ -46,6 +46,7 @@ static verto_ev *setup_socket(char *sock_name, verto_ctx *vctx)
ev = verto_add_io(vctx, vflags, accept_sock_conn, sock_ctx->fd);
if (!ev) {
+ free(sock_ctx);
return NULL;
}

81
Reorder-functions.patch
View File

@ -1,81 +0,0 @@
From 6803191543fe0cea1f39c75e9f81bbe90ab8b053 Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Wed, 6 Mar 2019 15:06:14 -0500
Subject: [PATCH] Reorder functions
Keep related functions closer together like before
Signed-off-by: Simo Sorce <simo@redhat.com>
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Resolves: #242
(cherry picked from commit 6accc0afead574e11447447c949f2abcb1a34826)
---
src/client/gpm_common.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c
index 95a39ab..808f350 100644
--- a/src/client/gpm_common.c
+++ b/src/client/gpm_common.c
@@ -140,16 +140,6 @@ static void gpm_close_socket(struct gpm_ctx *gpmctx)
gpmctx->fd = -1;
}
-static void gpm_epoll_close(struct gpm_ctx *gpmctx)
-{
- if (gpmctx->epollfd < 0) {
- return;
- }
-
- close(gpmctx->epollfd);
- gpmctx->epollfd = -1;
-}
-
static void gpm_timer_close(struct gpm_ctx *gpmctx)
{
if (gpmctx->timerfd < 0) {
@@ -160,13 +150,6 @@ static void gpm_timer_close(struct gpm_ctx *gpmctx)
gpmctx->timerfd = -1;
}
-static int gpm_release_sock(struct gpm_ctx *gpmctx)
-{
- gpm_epoll_close(gpmctx);
- gpm_timer_close(gpmctx);
- return pthread_mutex_unlock(&gpmctx->lock);
-}
-
static int gpm_timer_setup(struct gpm_ctx *gpmctx, int timeout_seconds)
{
int ret;
@@ -197,6 +180,16 @@ static int gpm_timer_setup(struct gpm_ctx *gpmctx, int timeout_seconds)
return 0;
}
+static void gpm_epoll_close(struct gpm_ctx *gpmctx)
+{
+ if (gpmctx->epollfd < 0) {
+ return;
+ }
+
+ close(gpmctx->epollfd);
+ gpmctx->epollfd = -1;
+}
+
static int gpm_epoll_setup(struct gpm_ctx *gpmctx)
{
struct epoll_event ev;
@@ -224,6 +217,13 @@ static int gpm_epoll_setup(struct gpm_ctx *gpmctx)
return ret;
}
+static int gpm_release_sock(struct gpm_ctx *gpmctx)
+{
+ gpm_epoll_close(gpmctx);
+ gpm_timer_close(gpmctx);
+ return pthread_mutex_unlock(&gpmctx->lock);
+}
+
static int gpm_grab_sock(struct gpm_ctx *gpmctx)
{
int ret;

31
Update-docs-to-reflect-actual-behavior-of-krb5_princ.patch
View File

@ -1,31 +0,0 @@
From 9b2bf07af65f38f204eabb4d7622a4c86ce06789 Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Tue, 2 Oct 2018 14:52:18 -0400
Subject: [PATCH] Update docs to reflect actual behavior of krb5_principal
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
Merges: #235
(cherry picked from commit ffb05b5eeb9aa818690e5eece5ab6d84cf8fc24b)
---
man/gssproxy.conf.5.xml | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/man/gssproxy.conf.5.xml b/man/gssproxy.conf.5.xml
index 21c9653..a6a620b 100644
--- a/man/gssproxy.conf.5.xml
+++ b/man/gssproxy.conf.5.xml
@@ -308,7 +308,12 @@
<varlistentry>
<term>krb5_principal (string)</term>
<listitem>
- <para>The krb5 principal to be used by this service.</para>
+ <para>
+ The krb5 principal to be used preferred for this
+ service, if one isn't requested by the
+ application. Note that this does not enforce use
+ of this specific name; it only sets a default.
+ </para>
<para>Default: krb5_principal = </para>
</listitem>
</varlistentry>

149
Use-pthread-keys-for-thread-local-storage.patch
View File

@ -1,149 +0,0 @@
From 89dc0ee157caa4617d32fd72287849296d7fe26d 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)
---
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);

7
gssproxy.spec
View File

@ -1,7 +1,7 @@
Name: gssproxy
Version: 0.8.2
Release: 1%{?dist}
Release: 2%{?dist}
Summary: GSSAPI Proxy
License: MIT
@ -14,7 +14,7 @@ Source1: rwtab
%global gpstatedir %{_localstatedir}/lib/gssproxy
### Patches ###
Patch0: Avoid-uninitialized-free-when-allocating-buffers.patch
### Dependencies ###
Requires: krb5-libs >= 1.12.0
@ -110,6 +110,9 @@ install -m644 %{SOURCE1} $RPM_BUILD_ROOT/%{_sysconfdir}/rwtab.d/gssproxy
%systemd_postun_with_restart gssproxy.service
%changelog
* Wed May 01 2019 Robbie Harwood <rharwood@redhat.com> - 0.8.2-2
- Avoid uninitialized free when allocating buffers
* Thu Apr 18 2019 Robbie Harwood <rharwood@redhat.com> - 0.8.2-1
- New usptream version (0.8.2)