From 6048bc4f127b0e302ee5c972264b7909dec3f7fc Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Mar 06 2019 15:49:22 +0000
Subject: [PATCH 1/2] 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 th
thread stuck forever.

Signed-off-by: Simo Sorce <simo@redhat.com>

---

diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c
index c254280..117d912 100644
--- a/src/client/gpm_common.c
+++ b/src/client/gpm_common.c
@@ -139,6 +139,18 @@ static void gpm_close_socket(struct gpm_ctx *gpmctx)
     gpmctx->fd = -1;
 }
 
+static int gpm_epoll_setup(struct gpm_ctx *gpmctx);
+static void gpm_epoll_close(struct gpm_ctx *gpmctx);
+static int gpm_timer_setup(struct gpm_ctx *gpmctx, int timeout_seconds);
+static void gpm_timer_close(struct gpm_ctx *gpmctx);
+
+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;
@@ -163,17 +175,24 @@ static int gpm_grab_sock(struct gpm_ctx *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) {
-        pthread_mutex_unlock(&gpmctx->lock);
+        goto done;
     }
-    return ret;
-}
+    /* create epoll fd as well */
+    ret = gpm_epoll_setup(gpmctx);
 
-static int gpm_release_sock(struct gpm_ctx *gpmctx)
-{
-    return pthread_mutex_unlock(&gpmctx->lock);
+done:
+    if (ret) {
+        gpm_release_sock(gpmctx);
+    }
+    return ret;
 }
 
 static void gpm_timer_close(struct gpm_ctx *gpmctx)
@@ -530,11 +549,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 +775,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);
     }

From ec1a70fdd15684b322415b546b3595748075addb Mon Sep 17 00:00:00 2001
From: Simo Sorce <simo@redhat.com>
Date: Mar 06 2019 15:49:28 +0000
Subject: [PATCH 2/2] 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>

---

diff --git a/src/client/gpm_common.c b/src/client/gpm_common.c
index 117d912..fed960a 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 {
@@ -296,7 +297,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) {