summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJiffin Tony Thottan <jthottan@redhat.com>2016-11-21 18:08:14 +0530
committerRaghavendra G <rgowdapp@redhat.com>2017-01-09 20:54:03 -0800
commitec3322bc932d2277873dc337b19c88381df78106 (patch)
tree02aaa8d24eff427a658b363fada3ded5f85dc3c6
parentc2aa8b1e448b47f48f58bec28606c790fa54b7bc (diff)
dht/rename : Incase of failure remove linkto file properly
Generally linkto file is created using root user. Consider following case, a user is trying to rename a file which he is not permitted. So the rename fails with EACESS and when rename tries to cleanup the linkto file, it fails. The above issue happens when rename/00.t test executed on nfs-ganesha clients : Steps executed in script * create a file "abc" using root * rename the file "abc" to "xyz" using a non root user, it fails with EACESS * delete "abc" * create directory "abc" using root * again try ot rename "abc" to "xyz" using non root user, test hungs here which slowly leds to OOM kill of ganesha process RCA put forwarded by Du for OOM kill of ganesha Note that when we hit this bug, we've a scenario of a dentry being present as: * a linkto file on one subvol * a directory on rest of subvols When a lookup happens on the dentry in such a scenario, the control flow goes into an infinite loop of: dht_lookup_everywhere dht_lookup_everywhere_cbk dht_lookup_unlink_cbk dht_lookup_everywhere_done dht_lookup_directory (as local->dir_count > 0) dht_lookup_dir_cbk (sets to local->need_selfheal = 1 as the entry is a linkto file on one of the subvol) dht_lookup_everywhere (as need_selfheal = 1). This infinite loop can cause increased consumption of memory due to: 1) dht_lookup_directory assigns a new layout to local->layout unconditionally 2) Most of the functions in this loop do a stack_wind of various fops. This results in growing of call stack (note that call-stack is destroyed only after lookup response is received by fuse - which never happens in this case) Thanks Du for root causing the oom kill and Sushant for suggesting the fix Upstream reference : >Change-Id: I1e16bc14aa685542afbd21188426ecb61fd2689d >BUG: 1397052 >Signed-off-by: Jiffin Tony Thottan <jthottan@redhat.com> >Reviewed-on: http://review.gluster.org/15894 >NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> >CentOS-regression: Gluster Build System <jenkins@build.gluster.org> >Smoke: Gluster Build System <jenkins@build.gluster.org> >Reviewed-by: Raghavendra G <rgowdapp@redhat.com> >(cherry picked from commit 57d59f4be205ae0c7888758366dc0049bdcfe449) Change-Id: I1e16bc14aa685542afbd21188426ecb61fd2689d BUG: 1401032 Signed-off-by: Jiffin Tony Thottan <jthottan@redhat.com> Reviewed-on: http://review.gluster.org/16016 Smoke: Gluster Build System <jenkins@build.gluster.org> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> CentOS-regression: Gluster Build System <jenkins@build.gluster.org> Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
-rw-r--r--xlators/cluster/dht/src/dht-common.c8
-rw-r--r--xlators/cluster/dht/src/dht-rename.c8
2 files changed, 14 insertions, 2 deletions
diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c
index 83a2fe8..d94a78c 100644
--- a/xlators/cluster/dht/src/dht-common.c
+++ b/xlators/cluster/dht/src/dht-common.c
@@ -1262,6 +1262,7 @@ dht_lookup_unlink_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
local = (dht_local_t*)frame->local;
path = local->loc.path;
+ FRAME_SU_UNDO (frame, dht_local_t);
gf_msg (this->name, GF_LOG_INFO, 0,
DHT_MSG_UNLINK_LOOKUP_INFO, "lookup_unlink returned with "
@@ -1955,7 +1956,12 @@ unlock:
loc->path, subvol->name,
(local->hashed_subvol?
local->hashed_subvol->name : "<null>"));
-
+ /* *
+ * These stale files may be created using root
+ * user. Hence deletion will work only with
+ * root.
+ */
+ FRAME_SU_DO (frame, dht_local_t);
STACK_WIND (frame, dht_lookup_unlink_cbk,
subvol, subvol->fops->unlink, loc,
0, dict_req);
diff --git a/xlators/cluster/dht/src/dht-rename.c b/xlators/cluster/dht/src/dht-rename.c
index 4ed659b..c48753a 100644
--- a/xlators/cluster/dht/src/dht-rename.c
+++ b/xlators/cluster/dht/src/dht-rename.c
@@ -641,6 +641,7 @@ dht_rename_unlink_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
local = frame->local;
prev = cookie;
+ FRAME_SU_UNDO (frame, dht_local_t);
if (!local) {
gf_msg (this->name, GF_LOG_ERROR, 0,
DHT_MSG_INVALID_VALUE,
@@ -749,7 +750,12 @@ dht_rename_cleanup (call_frame_t *frame)
local->loc2.pargfid) == 0) {
DHT_MARKER_DONT_ACCOUNT(xattr_new);
}
-
+ /* *
+ * The link to file is created using root permission.
+ * Hence deletion should happen using root. Otherwise
+ * it will fail.
+ */
+ FRAME_SU_DO (frame, dht_local_t);
STACK_WIND (frame, dht_rename_unlink_cbk,
src_cached, src_cached->fops->unlink,
&local->loc2, 0, xattr_new);