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

Fix gssproxy blocking inside epoll_wait() due to kernel race

Browse Source
This commit is contained in:
Robbie Harwood 2019-03-18 13:55:29 -04:00
parent bb2f7fcd5b
commit cc54c05ec1
4 changed files with 309 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -0,0 +1,47 @@
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) {

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

@ -0,0 +1,174 @@
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);
}

81
Reorder-functions.patch Normal file
View File

@ -0,0 +1,81 @@
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;

8
gssproxy.spec
View File

@ -1,7 +1,7 @@
Name: gssproxy
Version: 0.8.0
Release: 10%{?dist}
Release: 11%{?dist}
Summary: GSSAPI Proxy
License: MIT
@ -20,6 +20,9 @@ Patch2: Always-choose-highest-requested-debug-level.patch
Patch3: Don-t-leak-sock_ctx-if-verto_add_io-fails.patch
Patch4: Use-pthread-keys-for-thread-local-storage.patch
Patch5: Update-docs-to-reflect-actual-behavior-of-krb5_princ.patch
Patch6: Close-epoll-fd-within-the-lock.patch
Patch7: Add-a-safety-timeout-to-epoll.patch
Patch8: Reorder-functions.patch
### Dependencies ###
Requires: krb5-libs >= 1.12.0
@ -115,6 +118,9 @@ install -m644 %{SOURCE1} $RPM_BUILD_ROOT/%{_sysconfdir}/rwtab.d/gssproxy
%systemd_postun_with_restart gssproxy.service
%changelog
* Mon Mar 18 2019 Robbie Harwood <rharwood@redhat.com> - 0.8.0-11
- Fix gssproxy blocking inside epoll_wait() due to kernel race
* Fri Feb 01 2019 Fedora Release Engineering <releng@fedoraproject.org> - 0.8.0-10
- Rebuilt for https://fedoraproject.org/wiki/Fedora_30_Mass_Rebuild