summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSoumya Koduri <skoduri@redhat.com>2018-11-11 22:53:07 +0530
committerShyamsundar Ranganathan <srangana@redhat.com>2018-12-26 16:59:17 +0000
commit9ab7053c1c1255403a7e330892523c35a4fa59e6 (patch)
treeba74868dda2502dfd63b4e4074be26ca386ae677
parent6df227148c5651bc321b86c147ea20f90944d1be (diff)
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#1655532 Signed-off-by: Soumya Koduri <skoduri@redhat.com>
-rw-r--r--xlators/features/leases/src/leases-internal.c40
-rw-r--r--xlators/features/leases/src/leases.h1
2 files changed, 38 insertions, 3 deletions
diff --git a/xlators/features/leases/src/leases-internal.c b/xlators/features/leases/src/leases-internal.c
index 51cfbaeeec4..7202743c74b 100644
--- a/xlators/features/leases/src/leases-internal.c
+++ b/xlators/features/leases/src/leases-internal.c
@@ -673,6 +673,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) {
@@ -707,6 +708,15 @@ __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) {
@@ -793,6 +803,16 @@ 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);
@@ -813,6 +833,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;
}
@@ -846,7 +869,9 @@ recall_lease_timer_handler (struct gf_tw_timer_list *timer,
pthread_cond_broadcast (&priv->cond);
}
out:
- pthread_mutex_unlock (&priv->mutex);
+ /* unref the inode_ref taken by timer_data in __recall_lease */
+ inode_unref(timer_data->inode);
+ pthread_mutex_unlock(&priv->mutex);
GF_FREE (timer);
}
@@ -1173,6 +1198,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;
}
@@ -1217,9 +1243,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);
@@ -1227,6 +1253,7 @@ cleanup_client_leases (xlator_t *this, const char *client_uid)
l_inode = tmp1 = NULL;
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;
@@ -1240,6 +1267,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,
@@ -1257,6 +1288,7 @@ __remove_all_leases (xlator_t *this, lease_inode_ctx_t *lease_ctx)
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
@@ -1346,7 +1378,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 80dbcbb06b9..2e5ef12e1ba 100644
--- a/xlators/features/leases/src/leases.h
+++ b/xlators/features/leases/src/leases.h
@@ -186,6 +186,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