diff --git a/Add-a-safety-timeout-to-epoll.patch b/Add-a-safety-timeout-to-epoll.patch new file mode 100644 index 0000000..2635970 --- /dev/null +++ b/Add-a-safety-timeout-to-epoll.patch @@ -0,0 +1,47 @@ +From 616620ad1f8568437da8d9ec235ffa2f2ec9ca32 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +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 +[rharwood@redhat.com: remove outdated comment] +Reviewed-by: Robbie Harwood +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) { diff --git a/Close-epoll-fd-within-the-lock.patch b/Close-epoll-fd-within-the-lock.patch new file mode 100644 index 0000000..156c13e --- /dev/null +++ b/Close-epoll-fd-within-the-lock.patch @@ -0,0 +1,174 @@ +From 9c0c6b8202ecd20dc25ea37bab7b7297cfca077f Mon Sep 17 00:00:00 2001 +From: Simo Sorce +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 +[rharwood@redhat.com: cleanup commit message, adjusted function ordering] +Reviewed-by: Robbie Harwood +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); + } diff --git a/Reorder-functions.patch b/Reorder-functions.patch new file mode 100644 index 0000000..88027d9 --- /dev/null +++ b/Reorder-functions.patch @@ -0,0 +1,81 @@ +From 6803191543fe0cea1f39c75e9f81bbe90ab8b053 Mon Sep 17 00:00:00 2001 +From: Simo Sorce +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 +Reviewed-by: Robbie Harwood +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; diff --git a/gssproxy.spec b/gssproxy.spec index da4bb76..e88db20 100644 --- a/gssproxy.spec +++ b/gssproxy.spec @@ -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 - 0.8.0-11 +- Fix gssproxy blocking inside epoll_wait() due to kernel race + * Fri Feb 01 2019 Fedora Release Engineering - 0.8.0-10 - Rebuilt for https://fedoraproject.org/wiki/Fedora_30_Mass_Rebuild