leases: Fix incorrect inode_ref/unrefs

From testing & code-reading, found couple of places where
we incorrectly unref the inode resulting in use_after_free
crash or ref leaks. This patch addresses couple of them.

a) When we try to grant the very first lease for a inode,
inode_ref is taken in __add_lease. This ref should be active
till all the leases granted to that inode are released (i.e,
till lease_cnt > 0). In addition even after lease_cnt becomes '0',
the inode should be active till all the blocked fops are resumed.

Hence release this ref, after resuming all those fops. To avoid
granting new leases while resuming those fops, defined a new boolean
(blocked_fops_resuming) to flag it in the lease_ctx.

b) 'new_lease_inode' which creates new lease_inode_entry and
takes ref on inode, is used while adding that entry to
client_list and recall_list.

Use its counter function '__destroy_lease_inode' which does unref
while removing those entries from those lists.

c) inode ref is also taken when added to timer->data. Unref the same
after processing timer->data.

Change-Id: Ie77c78ff4a971e0d9a66178597fb34faf39205fb
updates: bz#1651323
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
(cherry picked from commit b7aec05aa965202ab73120acf0da4c32fe0cf16c)

2 files changed, 37 insertions, 3 deletions

diff --git a/xlators/features/leases/src/leases-internal.c b/xlators/features/leases/src/leases-internal.c
index 296799b8dff..00fb670ff2a 100644
--- a/xlators/features/leases/src/leases-internal.c
+++ b/xlators/features/leases/src/leases-internal.c
@@ -658,6 +658,7 @@ __remove_lease(xlator_t *this, inode_t *inode, lease_inode_ctx_t *lease_ctx,
             remove_from_clnt_list(this, client_uid, lease_ctx->inode);
         }
         __destroy_lease_id_entry(lease_entry);
+        lease_ctx->blocked_fops_resuming = _gf_true;
     }
 
     if (lease_ctx->lease_cnt == 0 && lease_ctx->timer) {
@@ -692,6 +693,14 @@ __is_lease_grantable(xlator_t *this, lease_inode_ctx_t *lease_ctx,
         goto out;
     }
 
+    if (lease_ctx->blocked_fops_resuming) {
+        gf_msg_debug(this->name, 0,
+                     "Previously blocked fops resuming, hence "
+                     "failing the lease request");
+        grant = _gf_false;
+        goto out;
+    }
+
     LOCK(&inode->lock);
     {
         list_for_each_entry(iter_fd, &inode->fd_list, inode_list)
@@ -776,6 +785,17 @@ do_blocked_fops(xlator_t *this, lease_inode_ctx_t *lease_ctx)
 
     pthread_mutex_lock(&lease_ctx->lock);
     {
+        if (!lease_ctx->blocked_fops_resuming) {
+            /* lease_ctx->blocked_fops_resuming will be set
+             * only when the last lease is released. That
+             * is when we need to resume blocked fops and unref
+             * the inode taken in __add_lease (when lease_cnt == 1).
+             * Return otherwise.
+             */
+            pthread_mutex_unlock(&lease_ctx->lock);
+            return;
+        }
+
         list_for_each_entry_safe(blk_fop, tmp, &lease_ctx->blocked_list, list)
         {
             list_del_init(&blk_fop->list);
@@ -797,6 +817,9 @@ do_blocked_fops(xlator_t *this, lease_inode_ctx_t *lease_ctx)
     pthread_mutex_lock(&lease_ctx->lock);
     {
         lease_ctx->lease_type = NONE;
+        /* unref the inode taken in __add_lease
+         * (when lease_cnt == 1) */
+        lease_ctx->blocked_fops_resuming = _gf_false;
         inode_unref(lease_ctx->inode);
         lease_ctx->inode = NULL;
     }
@@ -829,6 +852,8 @@ recall_lease_timer_handler(struct gf_tw_timer_list *timer, void *data,
         pthread_cond_broadcast(&priv->cond);
     }
 out:
+    /* unref the inode_ref taken by timer_data in __recall_lease */
+    inode_unref(timer_data->inode);
     pthread_mutex_unlock(&priv->mutex);
 
     GF_FREE(timer);
@@ -1166,6 +1191,7 @@ remove_clnt_leases(const char *client_uid, inode_t *inode, xlator_t *this)
                 lease_ctx->lease_cnt -= lease_entry->lease_cnt;
                 __destroy_lease_id_entry(lease_entry);
                 if (lease_ctx->lease_cnt == 0) {
+                    lease_ctx->blocked_fops_resuming = _gf_true;
                     pthread_mutex_unlock(&lease_ctx->lock);
                     goto unblock;
                 }
@@ -1212,9 +1238,9 @@ cleanup_client_leases(xlator_t *this, const char *client_uid)
                     list_del_init(&l_inode->list);
                     list_add_tail(&l_inode->list, &cleanup_list);
                 }
+                __destroy_lease_client(clnt);
                 break;
             }
-            __destroy_lease_client(clnt);
         }
     }
     pthread_mutex_unlock(&priv->mutex);
@@ -1223,6 +1249,7 @@ cleanup_client_leases(xlator_t *this, const char *client_uid)
     list_for_each_entry_safe(l_inode, tmp1, &cleanup_list, list)
     {
         remove_clnt_leases(client_uid, l_inode->inode, this);
+        __destroy_lease_inode(l_inode);
     }
 out:
     return ret;
@@ -1235,6 +1262,10 @@ __remove_all_leases(xlator_t *this, lease_inode_ctx_t *lease_ctx)
     lease_id_entry_t *lease_entry = NULL;
     lease_id_entry_t *tmp = NULL;
 
+    if (lease_ctx->lease_cnt == 0) {
+        /* No leases to remove. Return */
+        return;
+    }
     __dump_leases_info(this, lease_ctx);
 
     list_for_each_entry_safe(lease_entry, tmp, &lease_ctx->lease_id_list,
@@ -1250,8 +1281,8 @@ __remove_all_leases(xlator_t *this, lease_inode_ctx_t *lease_ctx)
     lease_ctx->lease_type = 0;
     lease_ctx->lease_cnt = 0;
     lease_ctx->recall_in_progress = _gf_false;
-    inode_unref(lease_ctx->inode);
     lease_ctx->timer = NULL;
+    lease_ctx->blocked_fops_resuming = _gf_true;
 
     /* TODO:
      * - Mark the corresponding fd bad. Could be done on client side
@@ -1342,7 +1373,9 @@ expired_recall_cleanup(void *data)
                          " hence cleaning up leases on the inode",
                          recall_entry->inode);
             remove_all_leases(this, recall_entry->inode);
-            list_del_init(&recall_entry->list);
+            /* no need to take priv->mutex lock as this entry
+             * reference is removed from global recall list. */
+            __destroy_lease_inode(recall_entry);
         }
     }
 
diff --git a/xlators/features/leases/src/leases.h b/xlators/features/leases/src/leases.h
index d5fc451289d..6ac712b0bb0 100644
--- a/xlators/features/leases/src/leases.h
+++ b/xlators/features/leases/src/leases.h
@@ -185,6 +185,7 @@ struct _lease_inode_ctx {
     uint64_t lease_cnt;              /* Total number of leases on this inode */
     uint64_t openfd_cnt;             /* number of fds open */
     gf_boolean_t recall_in_progress; /* if lease recall is sent on this inode */
+    gf_boolean_t blocked_fops_resuming; /* if blocked fops are being resumed */
     struct list_head blocked_list;   /* List of fops blocked until the
                                         lease recall is complete */
     inode_t *inode;                  /* this represents the inode on which the