233933
From bd553499909d2d57fd05696dc7604901cef3a36a Mon Sep 17 00:00:00 2001
233933
From: Soumya Koduri <skoduri@redhat.com>
233933
Date: Fri, 7 Jun 2019 17:20:15 +0530
233933
Subject: [PATCH 200/221] gfapi: fix incorrect initialization of upcall syncop
233933
 arguments
233933
233933
While sending upcall notifications via synctasks, the argument used to
233933
carry relevant data for these tasks is not initialized properly. This patch
233933
is to fix the same.
233933
233933
This is backport of below upstream fix -
233933
mainline: https://review.gluster.org/22839
233933
release-6: https://review.gluster.org/22871
233933
233933
Change-Id: I9fa8f841e71d3c37d3819fbd430382928c07176c
233933
fixes: bz#1717784
233933
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
233933
Reviewed-on: https://code.engineering.redhat.com/gerrit/173508
233933
Tested-by: RHGS Build Bot <nigelb@redhat.com>
233933
Reviewed-by: Kaleb Keithley <kkeithle@redhat.com>
233933
---
233933
 api/src/glfs-fops.c | 109 ++++++++++++++++++++++++++++++++++------------------
233933
 1 file changed, 72 insertions(+), 37 deletions(-)
233933
233933
diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c
233933
index 01ba60b..396f18c 100644
233933
--- a/api/src/glfs-fops.c
233933
+++ b/api/src/glfs-fops.c
233933
@@ -34,7 +34,7 @@
233933
 
233933
 struct upcall_syncop_args {
233933
     struct glfs *fs;
233933
-    struct gf_upcall *upcall_data;
233933
+    struct glfs_upcall *up_arg;
233933
 };
233933
 
233933
 #define READDIRBUF_SIZE (sizeof(struct dirent) + GF_NAME_MAX + 1)
233933
@@ -5714,12 +5714,28 @@ out:
233933
 }
233933
 
233933
 static int
233933
+upcall_syncop_args_free(struct upcall_syncop_args *args)
233933
+{
233933
+    if (args && args->up_arg)
233933
+        GLFS_FREE(args->up_arg);
233933
+    GF_FREE(args);
233933
+    return 0;
233933
+}
233933
+
233933
+static int
233933
 glfs_upcall_syncop_cbk(int ret, call_frame_t *frame, void *opaque)
233933
 {
233933
     struct upcall_syncop_args *args = opaque;
233933
 
233933
-    GF_FREE(args->upcall_data);
233933
-    GF_FREE(args);
233933
+    /* Here we not using upcall_syncop_args_free as application
233933
+     * will be cleaning up the args->up_arg using glfs_free
233933
+     * post processing upcall.
233933
+     */
233933
+    if (ret) {
233933
+        upcall_syncop_args_free(args);
233933
+    } else
233933
+        GF_FREE(args);
233933
+
233933
     return 0;
233933
 }
233933
 
233933
@@ -5727,13 +5743,29 @@ static int
233933
 glfs_cbk_upcall_syncop(void *opaque)
233933
 {
233933
     struct upcall_syncop_args *args = opaque;
233933
-    int ret = -1;
233933
     struct glfs_upcall *up_arg = NULL;
233933
     struct glfs *fs;
233933
-    struct gf_upcall *upcall_data;
233933
 
233933
     fs = args->fs;
233933
-    upcall_data = args->upcall_data;
233933
+    up_arg = args->up_arg;
233933
+
233933
+    if (fs->up_cbk && up_arg) {
233933
+        (fs->up_cbk)(up_arg, fs->up_data);
233933
+        return 0;
233933
+    }
233933
+
233933
+    return -1;
233933
+}
233933
+
233933
+static struct upcall_syncop_args *
233933
+upcall_syncop_args_init(struct glfs *fs, struct gf_upcall *upcall_data)
233933
+{
233933
+    struct upcall_syncop_args *args = NULL;
233933
+    int ret = -1;
233933
+    struct glfs_upcall *up_arg = NULL;
233933
+
233933
+    if (!fs || !upcall_data)
233933
+        goto out;
233933
 
233933
     up_arg = GLFS_CALLOC(1, sizeof(struct gf_upcall), glfs_release_upcall,
233933
                          glfs_mt_upcall_entry_t);
233933
@@ -5754,33 +5786,51 @@ glfs_cbk_upcall_syncop(void *opaque)
233933
             errno = EINVAL;
233933
     }
233933
 
233933
-    if (!ret && (up_arg->reason != GLFS_UPCALL_EVENT_NULL)) {
233933
-        /* It could so happen that the file which got
233933
-         * upcall notification may have got deleted by
233933
-         * the same client. In such cases up_arg->reason
233933
-         * is set to GLFS_UPCALL_EVENT_NULL. No need to
233933
-         * send upcall then */
233933
-        (fs->up_cbk)(up_arg, fs->up_data);
233933
-    } else if (up_arg->reason == GLFS_UPCALL_EVENT_NULL) {
233933
+    /* It could so happen that the file which got
233933
+     * upcall notification may have got deleted by
233933
+     * the same client. In such cases up_arg->reason
233933
+     * is set to GLFS_UPCALL_EVENT_NULL. No need to
233933
+     * send upcall then
233933
+     */
233933
+    if (up_arg->reason == GLFS_UPCALL_EVENT_NULL) {
233933
         gf_msg(THIS->name, GF_LOG_DEBUG, errno, API_MSG_INVALID_ENTRY,
233933
                "Upcall_EVENT_NULL received. Skipping it.");
233933
         goto out;
233933
-    } else {
233933
+    } else if (ret) {
233933
         gf_msg(THIS->name, GF_LOG_ERROR, errno, API_MSG_INVALID_ENTRY,
233933
                "Upcall entry validation failed.");
233933
         goto out;
233933
     }
233933
 
233933
+    args = GF_CALLOC(1, sizeof(struct upcall_syncop_args),
233933
+                     glfs_mt_upcall_entry_t);
233933
+    if (!args) {
233933
+        gf_msg(THIS->name, GF_LOG_ERROR, ENOMEM, API_MSG_ALLOC_FAILED,
233933
+               "Upcall syncop args allocation failed.");
233933
+        goto out;
233933
+    }
233933
+
233933
+    /* Note: we are not taking any ref on fs here.
233933
+     * Ideally applications have to unregister for upcall events
233933
+     * or stop polling for upcall events before performing
233933
+     * glfs_fini. And as for outstanding synctasks created, we wait
233933
+     * for all syncenv threads to finish tasks before cleaning up the
233933
+     * fs->ctx. Hence it seems safe to process these callback
233933
+     * notification without taking any lock/ref.
233933
+     */
233933
+    args->fs = fs;
233933
+    args->up_arg = up_arg;
233933
+
233933
     /* application takes care of calling glfs_free on up_arg post
233933
      * their processing */
233933
-    ret = 0;
233933
 
233933
+    return args;
233933
 out:
233933
-    if (ret && up_arg) {
233933
+    if (up_arg) {
233933
         GLFS_FREE(up_arg);
233933
     }
233933
 
233933
-    return 0;
233933
+    return NULL;
233933
 }
233933
 
233933
 static void
233933
@@ -5797,24 +5847,10 @@ glfs_cbk_upcall_data(struct glfs *fs, struct gf_upcall *upcall_data)
233933
         goto out;
233933
     }
233933
 
233933
-    args = GF_CALLOC(1, sizeof(struct upcall_syncop_args),
233933
-                     glfs_mt_upcall_entry_t);
233933
-    if (!args) {
233933
-        gf_msg(THIS->name, GF_LOG_ERROR, ENOMEM, API_MSG_ALLOC_FAILED,
233933
-               "Upcall syncop args allocation failed.");
233933
-        goto out;
233933
-    }
233933
+    args = upcall_syncop_args_init(fs, upcall_data);
233933
 
233933
-    /* Note: we are not taking any ref on fs here.
233933
-     * Ideally applications have to unregister for upcall events
233933
-     * or stop polling for upcall events before performing
233933
-     * glfs_fini. And as for outstanding synctasks created, we wait
233933
-     * for all syncenv threads to finish tasks before cleaning up the
233933
-     * fs->ctx. Hence it seems safe to process these callback
233933
-     * notification without taking any lock/ref.
233933
-     */
233933
-    args->fs = fs;
233933
-    args->upcall_data = gf_memdup(upcall_data, sizeof(*upcall_data));
233933
+    if (!args)
233933
+        goto out;
233933
 
233933
     ret = synctask_new(THIS->ctx->env, glfs_cbk_upcall_syncop,
233933
                        glfs_upcall_syncop_cbk, NULL, args);
233933
@@ -5823,8 +5859,7 @@ glfs_cbk_upcall_data(struct glfs *fs, struct gf_upcall *upcall_data)
233933
         gf_msg(THIS->name, GF_LOG_ERROR, errno, API_MSG_UPCALL_SYNCOP_FAILED,
233933
                "Synctak for Upcall event_type(%d) and gfid(%s) failed",
233933
                upcall_data->event_type, (char *)(upcall_data->gfid));
233933
-        GF_FREE(args->upcall_data);
233933
-        GF_FREE(args);
233933
+        upcall_syncop_args_free(args);
233933
     }
233933
 
233933
 out:
233933
-- 
233933
1.8.3.1
233933