|
 |
c530df |
From 32578afb817f20446d888326814b52a8f3d6c0fe Mon Sep 17 00:00:00 2001
|
|
|
c530df |
From: Simo Sorce <simo@redhat.com>
|
|
|
c530df |
Date: Thu, 26 Oct 2017 16:59:18 -0400
|
|
|
c530df |
Subject: [PATCH] Do not call gpm_grab_sock() twice
|
|
|
c530df |
|
|
|
c530df |
In the gpm_get_ctx() call, we unnecessarily call gpm_grab_sock() which
|
|
|
c530df |
would cause the lock to be held by one thread and never released. We
|
|
|
c530df |
already call gpm_grab_sock() as the first thing after gpm_get_ctx() in
|
|
|
c530df |
gpm_make_call(), plus gpm_make_call() properly releases the socket
|
|
|
c530df |
once done.
|
|
|
c530df |
|
|
|
c530df |
This corrects the deadlock fix in
|
|
|
c530df |
461a5fa9f91a2753ebeef6323a64239c35e2f250, which incorrectly released
|
|
|
c530df |
the lock we wanted to grab. This caused the socket to not be locked
|
|
|
c530df |
to our thread. Another thread could come along and change the global
|
|
|
c530df |
ctx while we were still using the socket from another thread, causing
|
|
|
c530df |
concurrency issues as only one request can be in flight on any given
|
|
|
c530df |
socket at the same time.
|
|
|
c530df |
|
|
|
c530df |
In special cases where the "thread" uid/gid changes (like in
|
|
|
c530df |
rpc.gssd), we end up closing the socket while we are still waiting for
|
|
|
c530df |
an answer from the server, causing additional issues and confusion.
|
|
|
c530df |
|
|
|
c530df |
[rharwood@redhat.com: squashed 2 commits; minor edits accordingly]
|
|
|
c530df |
Signed-off-by: Simo Sorce <simo@redhat.com>
|
|
|
c530df |
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
|
|
|
c530df |
Merges: #218
|
|
|
c530df |
(cherry picked from commit 8590c5dbc6fa07d0c366df23b982a4b6b9ffc259)
|
|
|
c530df |
---
|
|
|
c530df |
proxy/src/client/gpm_common.c | 9 +++------
|
|
|
c530df |
1 file changed, 3 insertions(+), 6 deletions(-)
|
|
|
c530df |
|
|
|
c530df |
diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c
|
|
|
c530df |
index 69f4741..2133618 100644
|
|
|
c530df |
--- a/proxy/src/client/gpm_common.c
|
|
|
c530df |
+++ b/proxy/src/client/gpm_common.c
|
|
|
c530df |
@@ -152,7 +152,9 @@ static int gpm_grab_sock(struct gpm_ctx *gpmctx)
|
|
|
c530df |
ret = gpm_open_socket(gpmctx);
|
|
|
c530df |
}
|
|
|
c530df |
|
|
|
c530df |
- pthread_mutex_unlock(&gpmctx->lock);
|
|
|
c530df |
+ if (ret) {
|
|
|
c530df |
+ pthread_mutex_unlock(&gpmctx->lock);
|
|
|
c530df |
+ }
|
|
|
c530df |
return ret;
|
|
|
c530df |
}
|
|
|
c530df |
|
|
|
c530df |
@@ -304,11 +306,6 @@ static struct gpm_ctx *gpm_get_ctx(void)
|
|
|
c530df |
|
|
|
c530df |
pthread_once(&gpm_init_once_control, gpm_init_once);
|
|
|
c530df |
|
|
|
c530df |
- ret = gpm_grab_sock(&gpm_global_ctx);
|
|
|
c530df |
- if (ret) {
|
|
|
c530df |
- return NULL;
|
|
|
c530df |
- }
|
|
|
c530df |
-
|
|
|
c530df |
return &gpm_global_ctx;
|
|
|
c530df |
}
|
|
|
c530df |
|