andy/glusterfs
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

performance/write-behind: fix use-after-free in readdirp

Browse Source 
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#1678570
(cherry picked from commit 64cc4458918e8c8bfdeb114da0a6501b2b98491a)
This commit is contained in:
Raghavendra Gowdappa 2019-02-13 17:08:14 +05:30 committed by Shyamsundar Ranganathan
parent 1f8b2912c8
commit bff265f8bc
1 changed files with 22 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
xlators/performance/write-behind/src/write-behind.c
View File

@ -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);
@ -2510,7 +2514,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:
@ -2552,16 +2557,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;
@ -2812,11 +2820,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;
}