From fa1718110e33cbcd2591a668b7f8a22d817e4ef5 Mon Sep 17 00:00:00 2001
From: Jiffin Tony Thottan <jthottan@redhat.com>
Date: Mon, 21 Nov 2016 18:08:14 +0530
Subject: [PATCH 220/227] 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>

Change-Id: I1e16bc14aa685542afbd21188426ecb61fd2689d
BUG: 1381452
Signed-off-by: Jiffin Tony Thottan <jthottan@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/92002
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
Tested-by: Atin Mukherjee <amukherj@redhat.com>
---
 xlators/cluster/dht/src/dht-common.c | 8 +++++++-
 xlators/cluster/dht/src/dht-rename.c | 8 +++++++-
 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 0001d73..ca84826 100644
--- a/xlators/cluster/dht/src/dht-common.c
+++ b/xlators/cluster/dht/src/dht-common.c
@@ -1128,6 +1128,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 "
@@ -1821,7 +1822,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 777c63d..a9ffd1d 100644
--- a/xlators/cluster/dht/src/dht-rename.c
+++ b/xlators/cluster/dht/src/dht-rename.c
@@ -637,6 +637,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,
@@ -745,7 +746,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);
-- 
2.9.3