From 69e1baa7d7f271d294e619148b4a2bae10230b5e 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)

Squashed with:

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)

(cherry picked from commit c33de0c213d570f370fd954869c2ad99901b2cf3)

[rharwood@redhat.com: I learned an important lesson upstream]

---

 proxy/src/client/gpm_common.c | 100 ++++++++++++++++++----------------

 1 file changed, 54 insertions(+), 46 deletions(-)

diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c

index b482efb..554c91c 100644

--- a/proxy/src/client/gpm_common.c

+++ b/proxy/src/client/gpm_common.c

@@ -139,44 +139,8 @@ static void gpm_close_socket(struct gpm_ctx *gpmctx)

     gpmctx->fd = -1;

 }

-static int gpm_grab_sock(struct gpm_ctx *gpmctx)

+static void gpm_timer_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);

-    }

-    return ret;

-}

-

-static int gpm_release_sock(struct gpm_ctx *gpmctx)

-{

-    return pthread_mutex_unlock(&gpmctx->lock);

-}

-

-static void gpm_timer_close(struct gpm_ctx *gpmctx) {

     if (gpmctx->timerfd < 0) {

         return;

     }

@@ -248,7 +212,59 @@ static int gpm_epoll_setup(struct gpm_ctx *gpmctx) {

     return ret;

 }

-static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags) {

+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;

+    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;

     int epoll_ret;

     struct epoll_event ev;

@@ -526,11 +542,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);

@@ -757,9 +768,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);

     }