This is an automated email from the git hooks/post-receive script.
rharwood pushed a change to branch master in repository gssproxy.
from 9e38cd5 Move run_as_user check out of drop_privs() new 0ccfd32 Close epoll fd within the lock new d55be9f Add a safety timeout to epoll
The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
Summary of changes: src/client/gpm_common.c | 111 +++++++++++++++++++++++++----------------------- 1 file changed, 58 insertions(+), 53 deletions(-)
This is an automated email from the git hooks/post-receive script.
rharwood pushed a commit to branch master in repository gssproxy.
commit 0ccfd32f8ef16caf65698c5319dfa251d43433af Author: Simo Sorce simo@redhat.com AuthorDate: Wed Mar 6 10:31:13 2019 -0500
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 --- 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; - } - - /* 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); + if (gpmctx->epollfd < 0) { + return; } - 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); }
This is an automated email from the git hooks/post-receive script.
rharwood pushed a commit to branch master in repository gssproxy.
commit d55be9fa2455fe52b6eb904ad427f22141ab3f26 Author: Simo Sorce simo@redhat.com AuthorDate: Wed Mar 6 10:36:11 2019 -0500
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 --- 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) {
gss-proxy@lists.fedorahosted.org