summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRaghavendra Gowdappa <rgowdapp@redhat.com>2019-02-13 17:08:14 +0530
committerShyamsundar Ranganathan <srangana@redhat.com>2019-02-22 14:50:00 +0000
commit3c1368fda17b71a95edf1f4fe102e15f64820e3e (patch)
tree871bba95ec691397d6ea88e9dc7fb182a1d31bd7
parentceee95dbc3c80d52134ceaee22e17c9f1c861300 (diff)
performance/write-behind: fix use-after-free in readdirp
Two issues were found: 1. in wb_readdirp_cbk, inode should unrefed after wb_inode is unlocked. Otherwise, inode and hence the context wb_inode can be freed by the type we try to unlock wb_inode 2. wb_readdirp_mark_end iterates over a list of wb_inodes of children of a directory. But inodes could've been freed and hence the list might be corrupted. To fix take a reference on inode before adding it to invalidate_list of parent. Change-Id: I911b0e0b2060f7f41ded0b05db11af6f9b7c09c5 Signed-off-by: Raghavendra Gowdappa <rgowdapp@redhat.com> Updates: bz#1671556
-rw-r--r--xlators/performance/write-behind/src/write-behind.c40
1 files changed, 22 insertions, 18 deletions
diff --git a/xlators/performance/write-behind/src/write-behind.c b/xlators/performance/write-behind/src/write-behind.c
index 7e4bb6a3c83..60c9148fc30 100644
--- a/xlators/performance/write-behind/src/write-behind.c
+++ b/xlators/performance/write-behind/src/write-behind.c
@@ -226,7 +226,7 @@ out:
}
static void
-wb_set_invalidate(wb_inode_t *wb_inode, int set)
+wb_set_invalidate(wb_inode_t *wb_inode)
{
int readdirps = 0;
inode_t *parent_inode = NULL;
@@ -240,21 +240,21 @@ wb_set_invalidate(wb_inode_t *wb_inode, int set)
LOCK(&wb_parent_inode->lock);
{
readdirps = GF_ATOMIC_GET(wb_parent_inode->readdirps);
- if (readdirps && set) {
- GF_ATOMIC_SWAP(wb_inode->invalidate, 1);
- list_del_init(&wb_inode->invalidate_list);
+ if (readdirps && list_empty(&wb_inode->invalidate_list)) {
+ inode_ref(wb_inode->inode);
+ GF_ATOMIC_INIT(wb_inode->invalidate, 1);
list_add(&wb_inode->invalidate_list,
&wb_parent_inode->invalidate_list);
- } else if (readdirps == 0) {
- GF_ATOMIC_SWAP(wb_inode->invalidate, 0);
- list_del_init(&wb_inode->invalidate_list);
}
}
UNLOCK(&wb_parent_inode->lock);
} else {
- GF_ATOMIC_SWAP(wb_inode->invalidate, 0);
+ GF_ATOMIC_INIT(wb_inode->invalidate, 0);
}
+ if (parent_inode)
+ inode_unref(parent_inode);
+
return;
}
@@ -718,6 +718,10 @@ wb_inode_destroy(wb_inode_t *wb_inode)
{
GF_VALIDATE_OR_GOTO("write-behind", wb_inode, out);
+ GF_ASSERT(list_empty(&wb_inode->todo));
+ GF_ASSERT(list_empty(&wb_inode->liability));
+ GF_ASSERT(list_empty(&wb_inode->temptation));
+
LOCK_DESTROY(&wb_inode->lock);
GF_FREE(wb_inode);
out:
@@ -1092,7 +1096,7 @@ wb_fulfill_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
* In the above scenario, stat for the file is sent back in readdirp
* response but it is stale.
* </comment> */
- wb_set_invalidate(wb_inode, 1);
+ wb_set_invalidate(wb_inode);
if (op_ret == -1) {
wb_fulfill_err(head, op_errno);
@@ -2507,7 +2511,8 @@ wb_mark_readdirp_end(xlator_t *this, inode_t *directory)
invalidate_list)
{
list_del_init(&wb_inode->invalidate_list);
- GF_ATOMIC_SWAP(wb_inode->invalidate, 0);
+ GF_ATOMIC_INIT(wb_inode->invalidate, 0);
+ inode_unref(wb_inode->inode);
}
}
unlock:
@@ -2549,16 +2554,19 @@ wb_readdirp_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
entry->inode = NULL;
memset(&entry->d_stat, 0, sizeof(entry->d_stat));
-
- inode_unref(inode);
}
}
UNLOCK(&wb_inode->lock);
+
+ if (inode) {
+ inode_unref(inode);
+ inode = NULL;
+ }
}
+unwind:
wb_mark_readdirp_end(this, fd->inode);
-unwind:
frame->local = NULL;
STACK_UNWIND_STRICT(readdirp, frame, op_ret, op_errno, entries, xdata);
return 0;
@@ -2809,11 +2817,7 @@ wb_forget(xlator_t *this, inode_t *inode)
if (!wb_inode)
return 0;
- GF_ASSERT(list_empty(&wb_inode->todo));
- GF_ASSERT(list_empty(&wb_inode->liability));
- GF_ASSERT(list_empty(&wb_inode->temptation));
-
- GF_FREE(wb_inode);
+ wb_inode_destroy(wb_inode);
return 0;
}