From d10c5375b33520f36fd6acbd47b617d43f529ca2 Mon Sep 17 00:00:00 2001 From: Poornima G Date: Wed, 15 Feb 2017 11:18:31 +0530 Subject: libglusterfs: Fix a crash due to race between inode_ctx_set and inode_ref Issue: Currently inode ref count is guarded by inode_table->lock, and inode_ctx is guarded by inode->lock. With the new patch [1] inode_ref was modified to change the inode_ctx to track the ref count per xlator. Thus inode_ref performed under inode_table->lock is modifying inode_ctx which has to be modified only under inode->lock Solution: When a inode is created, inode_ctx holder is allocated for all the xlators. Hence in case of inode_ctx_set instead of using the first free index in inode ctx holder, we can have predecided index for every xlator in the graph. Credits Pranith K [1] http://review.gluster.org/13736 > Reviewed-on: https://review.gluster.org/16622 > Smoke: Gluster Build System > NetBSD-regression: NetBSD Build System > Reviewed-by: Niels de Vos > CentOS-regression: Gluster Build System > Reviewed-by: Pranith Kumar Karampuri Change-Id: I1bfe111c211fcc4fcd761bba01dc87c4c69b5170 BUG: 1423385 Signed-off-by: Poornima G Reviewed-on: https://review.gluster.org/16655 NetBSD-regression: NetBSD Build System Smoke: Gluster Build System CentOS-regression: Gluster Build System Reviewed-by: Shyamsundar Ranganathan --- libglusterfs/src/graph.c | 1 + libglusterfs/src/graph.y | 1 + libglusterfs/src/inode.c | 48 ++++++++++++++--------------------------------- libglusterfs/src/xlator.h | 3 +++ 4 files changed, 19 insertions(+), 34 deletions(-) (limited to 'libglusterfs/src') diff --git a/libglusterfs/src/graph.c b/libglusterfs/src/graph.c index 47f473a580f..0c7d6cf6101 100644 --- a/libglusterfs/src/graph.c +++ b/libglusterfs/src/graph.c @@ -111,6 +111,7 @@ glusterfs_graph_set_first (glusterfs_graph_t *graph, xlator_t *xl) graph->first = xl; graph->xl_count++; + xl->xl_id = graph->xl_count; } diff --git a/libglusterfs/src/graph.y b/libglusterfs/src/graph.y index e6a26058a06..7df3479d701 100644 --- a/libglusterfs/src/graph.y +++ b/libglusterfs/src/graph.y @@ -182,6 +182,7 @@ new_volume (char *name) construct->first = curr; construct->xl_count++; + curr->xl_id = construct->xl_count; gf_msg_trace ("parser", 0, "New node for '%s'", name); diff --git a/libglusterfs/src/inode.c b/libglusterfs/src/inode.c index d39a2194921..573ec03819a 100644 --- a/libglusterfs/src/inode.c +++ b/libglusterfs/src/inode.c @@ -451,21 +451,15 @@ static int __inode_get_xl_index (inode_t *inode, xlator_t *xlator) { int set_idx = -1; - int index = 0; - - for (index = 0; index < inode->table->ctxcount; index++) { - if (!inode->_ctx[index].xl_key) { - if (set_idx == -1) - set_idx = index; - /* dont break, to check if key already exists - further on */ - } - if (inode->_ctx[index].xl_key == xlator) { - set_idx = index; - break; - } - } + if ((inode->_ctx[xlator->xl_id].xl_key != NULL) && + (inode->_ctx[xlator->xl_id].xl_key != xlator)) + goto out; + + set_idx = xlator->xl_id; + inode->_ctx[set_idx].xl_key = xlator; + +out: return set_idx; } @@ -2075,12 +2069,8 @@ __inode_ctx_get2 (inode_t *inode, xlator_t *xlator, uint64_t *value1, if (!inode || !xlator || !inode->_ctx) goto out; - for (index = 0; index < inode->table->ctxcount; index++) { - if (inode->_ctx[index].xl_key == xlator) - break; - } - - if (index == inode->table->ctxcount) + index = xlator->xl_id; + if (inode->_ctx[index].xl_key != xlator) goto out; if (inode->_ctx[index].value1) { @@ -2196,13 +2186,8 @@ inode_ctx_del2 (inode_t *inode, xlator_t *xlator, uint64_t *value1, if (!inode->_ctx) goto unlock; - for (index = 0; index < inode->table->ctxcount; - index++) { - if (inode->_ctx[index].xl_key == xlator) - break; - } - - if (index == inode->table->ctxcount) { + index = xlator->xl_id; + if (inode->_ctx[index].xl_key != xlator) { ret = -1; goto unlock; } @@ -2242,13 +2227,8 @@ __inode_ctx_reset2 (inode_t *inode, xlator_t *xlator, uint64_t *value1, LOCK (&inode->lock); { - for (index = 0; index < inode->table->ctxcount; - index++) { - if (inode->_ctx[index].xl_key == xlator) - break; - } - - if (index == inode->table->ctxcount) { + index = xlator->xl_id; + if (inode->_ctx[index].xl_key != xlator) { ret = -1; goto unlock; } diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h index 1e2698bb61f..c2959efbd95 100644 --- a/libglusterfs/src/xlator.h +++ b/libglusterfs/src/xlator.h @@ -953,6 +953,9 @@ struct _xlator { /* Saved volfile ID (used for multiplexing) */ char *volfile_id; + + /* Its used as an index to inode_ctx*/ + uint32_t xl_id; }; typedef struct { -- cgit