features/gfid-access: Fix inode leaks and loc path corruption

        Backport of http://review.gluster.org/8009
        Backport of http://review.gluster.org/8163

BUG: 1112659
Change-Id: Ic70a3ddfcfef88909c12ca6791a8e20c3fee2eae
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/8250
Reviewed-by: Kotresh HR <khiremat@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Niels de Vos <ndevos@redhat.com>

3 files changed, 207 insertions, 123 deletions

diff --git a/tests/basic/gfid-access.t b/tests/basic/gfid-access.t
new file mode 100644
index 00000000000..24477acba5c
--- /dev/null
+++ b/tests/basic/gfid-access.t
@@ -0,0 +1,60 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume create $V0 $H0:$B0/${V0}0
+TEST $CLI volume start $V0
+TEST glusterfs --volfile-id=/$V0 --volfile-server=$H0 $M0 --attribute-timeout=0 --entry-timeout=0 --aux-gfid-mount
+TEST mkdir $M0/a
+TEST touch $M0/b
+a_gfid_str=$(gf_gfid_xattr_to_str $(gf_get_gfid_xattr $B0/${V0}0/a))
+b_gfid_str=$(gf_gfid_xattr_to_str $(gf_get_gfid_xattr $B0/${V0}0/b))
+
+#Operations on Directory
+TEST setfattr -n trusted.abc -v abc $M0/a
+EXPECT "abc" echo $(getfattr -n trusted.abc $M0/a)
+EXPECT "abc" echo $(getfattr -n trusted.abc $M0/.gfid/$a_gfid_str)
+TEST setfattr -x trusted.abc $M0/a
+TEST ! getfattr -n trusted.abc $M0/a
+TEST ! getfattr -n trusted.abc $M0/.gfid/$a_gfid_str
+TEST chmod 0777 $M0/a
+EXPECT "777" stat -c "%a" $M0/a
+EXPECT "777" stat -c "%a" $M0/.gfid/$a_gfid_str
+
+TEST setfattr -n trusted.abc -v def $M0/.gfid/$a_gfid_str
+EXPECT "def" echo $(getfattr -n trusted.abc $M0/a)
+EXPECT "def" echo $(getfattr -n trusted.abc $M0/.gfid/$a_gfid_str)
+TEST setfattr -x trusted.abc $M0/.gfid/$a_gfid_str
+TEST ! getfattr -n trusted.abc $M0/a
+TEST ! getfattr -n trusted.abc $M0/.gfid/$a_gfid_str
+TEST chmod 0777 $M0/.gfid/$a_gfid_str
+EXPECT "777" stat -c "%a" $M0/a
+EXPECT "777" stat -c "%a" $M0/.gfid/$a_gfid_str
+
+#Operations on File
+TEST setfattr -n trusted.abc -v abc $M0/b
+EXPECT "abc" echo $(getfattr -n trusted.abc $M0/b)
+EXPECT "abc" echo $(getfattr -n trusted.abc $M0/.gfid/$b_gfid_str)
+TEST setfattr -x trusted.abc $M0/b
+TEST ! getfattr -n trusted.abc $M0/b
+TEST ! getfattr -n trusted.abc $M0/.gfid/$b_gfid_str
+TEST chmod 0777 $M0/b
+EXPECT "777" stat -c "%a" $M0/b
+EXPECT "777" stat -c "%a" $M0/.gfid/$b_gfid_str
+
+TEST setfattr -n trusted.abc -v def $M0/.gfid/$b_gfid_str
+EXPECT "def" echo $(getfattr -n trusted.abc $M0/b)
+EXPECT "def" echo $(getfattr -n trusted.abc $M0/.gfid/$b_gfid_str)
+TEST setfattr -x trusted.abc $M0/.gfid/$b_gfid_str
+TEST ! getfattr -n trusted.abc $M0/b
+TEST ! getfattr -n trusted.abc $M0/.gfid/$b_gfid_str
+TEST chmod 0777 $M0/.gfid/$b_gfid_str
+EXPECT "777" stat -c "%a" $M0/b
+EXPECT "777" stat -c "%a" $M0/.gfid/$b_gfid_str
+
+cleanup
diff --git a/xlators/features/gfid-access/src/gfid-access.c b/xlators/features/gfid-access/src/gfid-access.c
index a576e226660..217864782b7 100644
--- a/xlators/features/gfid-access/src/gfid-access.c
+++ b/xlators/features/gfid-access/src/gfid-access.c
@@ -17,6 +17,48 @@
 #include "byte-order.h"
 
 
+int
+ga_valid_inode_loc_copy (loc_t *dst, loc_t *src, xlator_t *this)
+{
+        int      ret        = 0;
+        uint64_t value      = 0;
+
+        /* if its an entry operation, on the virtual */
+        /* directory inode as parent, we need to handle */
+        /* it properly */
+        ret = loc_copy (dst, src);
+        if (ret < 0)
+                goto out;
+
+        if (dst->parent) {
+                ret = inode_ctx_get (dst->parent, this, &value);
+                if (ret < 0) {
+                        ret = 0; //real-inode
+                        goto out;
+                }
+                inode_unref (dst->parent);
+                dst->parent = inode_ref ((inode_t*)value);
+                /* if parent is virtual, no need to handle */
+                /* loc->inode */
+                goto out;
+        }
+
+        /* if its an inode operation, on the virtual */
+        /* directory inode itself, we need to handle */
+        /* it properly */
+        if (dst->inode) {
+                ret = inode_ctx_get (dst->inode, this, &value);
+                if (ret < 0) {
+                        ret = 0; //real-inode
+                        goto out;
+                }
+                inode_unref (dst->inode);
+                dst->inode = inode_ref ((inode_t*)value);
+                goto out;
+        }
+out:
+        return ret;
+}
 
 void
 ga_newfile_args_free (ga_newfile_args_t *args)
@@ -96,7 +138,6 @@ ga_newfile_parse_args (xlator_t *this, data_t *data)
         blob_len -= sizeof (uint32_t);
 
         len = strnlen (blob, blob_len);
-        if (len == blob_len)
         if (len == blob_len) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "gfid: %s. No null byte present.",
@@ -277,7 +318,11 @@ ga_fill_tmp_loc (loc_t *loc, xlator_t *this, uuid_t gfid,
                 new_loc->inode = inode_new (parent->table);
 
         loc_path (new_loc, bname);
-        new_loc->name = basename (new_loc->path);
+        if (new_loc->path) {
+                new_loc->name = strrchr (new_loc->path, '/');
+                if (new_loc->name)
+                        new_loc->name++;
+        }
 
         gfid_ptr = GF_CALLOC (1, sizeof(uuid_t), gf_common_mt_char);
         if (!gfid_ptr) {
@@ -571,11 +616,10 @@ int32_t
 ga_setxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *dict,
              int32_t flags, dict_t *xdata)
 {
-
         data_t  *data     = NULL;
         int      op_errno = ENOMEM;
         int      ret      = 0;
-        inode_t *unref    = NULL;
+        loc_t   ga_loc    = {0, };
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
 
@@ -597,15 +641,15 @@ ga_setxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *dict,
 
         //If the inode is a virtual inode change the inode otherwise perform
         //the operation on same inode
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, ga_setxattr_cbk, FIRST_CHILD(this),
-                    FIRST_CHILD(this)->fops->setxattr, loc, dict, flags,
+                    FIRST_CHILD(this)->fops->setxattr, &ga_loc, dict, flags,
                     xdata);
-        if (unref)
-                inode_unref (unref);
 
+        loc_wipe (&ga_loc);
         return 0;
 err:
         STACK_UNWIND_STRICT (setxattr, frame, -1, op_errno, xdata);
@@ -888,7 +932,7 @@ int
 ga_mkdir (call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode,
           mode_t umask, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
@@ -909,7 +953,7 @@ int
 ga_create (call_frame_t *frame, xlator_t *this, loc_t *loc, int flags,
            mode_t mode, mode_t umask, fd_t *fd, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
@@ -929,7 +973,7 @@ int
 ga_symlink (call_frame_t *frame, xlator_t *this, const char *linkname,
             loc_t *loc, mode_t umask, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
@@ -948,7 +992,7 @@ int
 ga_mknod (call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode,
           dev_t rdev, mode_t umask, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
@@ -968,20 +1012,21 @@ int
 ga_rmdir (call_frame_t *frame, xlator_t *this, loc_t *loc, int flag,
           dict_t *xdata)
 {
-        int op_errno = 0;
-        inode_t *unref = NULL;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_rmdir_cbk,
                     FIRST_CHILD(this), FIRST_CHILD(this)->fops->rmdir,
-                    loc, flag, xdata);
-        if (unref)
-                inode_unref (unref);
+                    &ga_loc, flag, xdata);
 
+        loc_wipe (&ga_loc);
         return 0;
 err:
         STACK_UNWIND_STRICT (rmdir, frame, -1, op_errno, NULL,
@@ -994,21 +1039,21 @@ int
 ga_unlink (call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t xflag,
            dict_t *xdata)
 {
-        int op_errno = 0;
-        inode_t *unref = NULL;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_ENTRY_OP_CHECK (loc, op_errno, err);
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_unlink_cbk,
                     FIRST_CHILD(this), FIRST_CHILD(this)->fops->unlink,
-                    loc, xflag, xdata);
-
-        if (unref)
-                inode_unref (unref);
+                    &ga_loc, xflag, xdata);
 
+        loc_wipe (&ga_loc);
         return 0;
 err:
         STACK_UNWIND_STRICT (unlink, frame, -1, op_errno, NULL,
@@ -1021,30 +1066,30 @@ int
 ga_rename (call_frame_t *frame, xlator_t *this,
            loc_t *oldloc, loc_t *newloc, dict_t *xdata)
 {
-        int op_errno = 0;
-        inode_t *oldloc_unref = NULL;
-        inode_t *newloc_unref = NULL;
+        int   op_errno  = ENOMEM;
+        int   ret       = 0;
+        loc_t ga_oldloc = {0, };
+        loc_t ga_newloc = {0, };
 
         GFID_ACCESS_ENTRY_OP_CHECK (oldloc, op_errno, err);
         GFID_ACCESS_ENTRY_OP_CHECK (newloc, op_errno, err);
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, oldloc, oldloc_unref,
-                                         handle_newloc);
+        ret = ga_valid_inode_loc_copy (&ga_oldloc, oldloc, this);
+        if (ret < 0)
+                goto err;
 
-handle_newloc:
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, newloc, newloc_unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_newloc, newloc, this);
+        if (ret < 0) {
+                loc_wipe (&ga_oldloc);
+                goto err;
+        }
 
-wind:
         STACK_WIND (frame, default_rename_cbk,
                     FIRST_CHILD(this), FIRST_CHILD(this)->fops->rename,
-                    oldloc, newloc, xdata);
-
-        if (oldloc_unref)
-                inode_unref (oldloc_unref);
-
-        if (newloc_unref)
-                inode_unref (newloc_unref);
+                    &ga_oldloc, &ga_newloc, xdata);
 
+        loc_wipe (&ga_newloc);
+        loc_wipe (&ga_oldloc);
         return 0;
 err:
         STACK_UNWIND_STRICT (rename, frame, -1, op_errno, NULL,
@@ -1058,31 +1103,32 @@ int
 ga_link (call_frame_t *frame, xlator_t *this,
          loc_t *oldloc, loc_t *newloc, dict_t *xdata)
 {
-        int op_errno = 0;
-        inode_t *oldloc_unref = NULL;
-        inode_t *newloc_unref = NULL;
+        int   op_errno  = ENOMEM;
+        int   ret       = 0;
+        loc_t ga_oldloc = {0, };
+        loc_t ga_newloc = {0, };
 
         GFID_ACCESS_ENTRY_OP_CHECK (oldloc, op_errno, err);
         GFID_ACCESS_ENTRY_OP_CHECK (newloc, op_errno, err);
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, oldloc, oldloc_unref,
-                                         handle_newloc);
+        ret = ga_valid_inode_loc_copy (&ga_oldloc, oldloc, this);
+        if (ret < 0)
+                goto err;
 
-handle_newloc:
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, newloc, newloc_unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_newloc, newloc, this);
+        if (ret < 0) {
+                loc_wipe (&ga_oldloc);
+                goto err;
+        }
 
-wind:
         STACK_WIND (frame, default_link_cbk,
                     FIRST_CHILD(this), FIRST_CHILD(this)->fops->link,
-                    oldloc, newloc, xdata);
-
-        if (oldloc_unref)
-                inode_unref (oldloc_unref);
-
-        if (newloc_unref)
-                inode_unref (newloc_unref);
+                    &ga_oldloc, &ga_newloc, xdata);
 
+        loc_wipe (&ga_newloc);
+        loc_wipe (&ga_oldloc);
         return 0;
+
 err:
         STACK_UNWIND_STRICT (link, frame, -1, op_errno, NULL,
                              NULL, NULL, NULL, xdata);
@@ -1094,7 +1140,7 @@ int32_t
 ga_opendir (call_frame_t *frame, xlator_t *this, loc_t *loc,
             fd_t *fd, dict_t *xdata)
 {
-        int op_errno = 0;
+        int op_errno = ENOMEM;
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
 
@@ -1120,23 +1166,24 @@ int32_t
 ga_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
              const char *name, dict_t *xdata)
 {
-        inode_t     *unref     = NULL;
-        int         op_errno   = 0;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_getxattr_cbk, FIRST_CHILD(this),
-                    FIRST_CHILD(this)->fops->getxattr, loc, name, xdata);
+                    FIRST_CHILD(this)->fops->getxattr, &ga_loc, name, xdata);
 
-        if (unref)
-                inode_unref (unref);
+        loc_wipe (&ga_loc);
 
         return 0;
-
 err:
         STACK_UNWIND_STRICT (getxattr, frame, -1, op_errno, NULL, xdata);
+
         return 0;
 }
 
@@ -1144,26 +1191,30 @@ int32_t
 ga_stat (call_frame_t *frame, xlator_t *this, loc_t *loc,
          dict_t *xdata)
 {
-        inode_t          *unref         = NULL;
-        ga_private_t     *priv          = NULL;
+        int          op_errno = ENOMEM;
+        int          ret      = -1;
+        loc_t        ga_loc   = {0, };
+        ga_private_t *priv    = NULL;
 
-        GF_ASSERT (this);
         priv = this->private;
-        GF_ASSERT (priv);
-
         /* If stat is on ".gfid" itself, do not wind further,
          * return fake stat and return success.
          */
         if (__is_gfid_access_dir(loc->gfid))
                 goto out;
 
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_stat_cbk, FIRST_CHILD(this),
-                    FIRST_CHILD(this)->fops->stat, loc, xdata);
-        if (unref)
-                inode_unref (unref);
+                    FIRST_CHILD(this)->fops->stat, &ga_loc, xdata);
+
+        loc_wipe (&ga_loc);
+        return 0;
+
+err:
+        STACK_UNWIND_STRICT (stat, frame, -1, op_errno, NULL, xdata);
 
         return 0;
 
@@ -1177,22 +1228,24 @@ ga_setattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
             struct iatt *stbuf, int32_t valid,
             dict_t *xdata)
 {
-        inode_t     *unref       = NULL;
-        int         op_errno     = 0;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_setattr_cbk, FIRST_CHILD (this),
-                    FIRST_CHILD (this)->fops->setattr, loc, stbuf, valid,
+                    FIRST_CHILD (this)->fops->setattr, &ga_loc, stbuf, valid,
                     xdata);
-        if (unref)
-                inode_unref (unref);
 
+        loc_wipe (&ga_loc);
         return 0;
 err:
         STACK_UNWIND_STRICT (setattr, frame, -1, op_errno, NULL, NULL, xdata);
+
         return 0;
 }
 
@@ -1200,22 +1253,25 @@ int32_t
 ga_removexattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
                 const char *name, dict_t *xdata)
 {
-        inode_t     *unref       = NULL;
-        int         op_errno     = 0;
+        int   op_errno = ENOMEM;
+        int   ret      = -1;
+        loc_t ga_loc   = {0, };
 
         GFID_ACCESS_INODE_OP_CHECK (loc, op_errno, err);
-        GFID_ACCESS_GET_VALID_DIR_INODE (this, loc, unref, wind);
+        ret = ga_valid_inode_loc_copy (&ga_loc, loc, this);
+        if (ret < 0)
+                goto err;
 
-wind:
         STACK_WIND (frame, default_removexattr_cbk, FIRST_CHILD(this),
-                    FIRST_CHILD(this)->fops->removexattr, loc, name,
+                    FIRST_CHILD(this)->fops->removexattr, &ga_loc, name,
                     xdata);
-        if (unref)
-                inode_unref (unref);
 
+        loc_wipe (&ga_loc);
         return 0;
+
 err:
         STACK_UNWIND_STRICT (removexattr, frame, -1, op_errno, xdata);
+
         return 0;
 }
 
diff --git a/xlators/features/gfid-access/src/gfid-access.h b/xlators/features/gfid-access/src/gfid-access.h
index 31c82e07891..5c7a95af4c8 100644
--- a/xlators/features/gfid-access/src/gfid-access.h
+++ b/xlators/features/gfid-access/src/gfid-access.h
@@ -31,38 +31,6 @@
 #define GF_GFID_DIR ".gfid"
 #define GF_AUX_GFID 0xd
 
-#define GFID_ACCESS_GET_VALID_DIR_INODE(x,l,unref,lbl) do {             \
-                int       ret       = 0;                                \
-                uint64_t  value     = 0;                                \
-                inode_t  *tmp_inode = NULL;                             \
-                                                                        \
-                /* if its an entry operation, on the virtual */         \
-                /* directory inode as parent, we need to handle */      \
-                /* it properly */                                       \
-                if (l->parent) {                                        \
-                        ret = inode_ctx_get (l->parent, x, &value);     \
-                        if (ret)                                        \
-                                goto lbl;                               \
-                        tmp_inode = (inode_t *)value;                   \
-                        l->parent = inode_ref (tmp_inode);                          \
-                        /* if parent is virtual, no need to handle */   \
-                        /* loc->inode */                                \
-                        break;                                          \
-                }                                                       \
-                                                                        \
-                /* if its an inode operation, on the virtual */         \
-                /* directory inode itself, we need to handle */         \
-                /* it properly */                                       \
-                if (l->inode) {                                         \
-                        ret = inode_ctx_get (l->inode, x, &value);      \
-                        if (ret)                                        \
-                                goto lbl;                               \
-                        tmp_inode = (inode_t *)value;                   \
-                        l->inode = inode_ref (tmp_inode);                           \
-                }                                                       \
-                                                                        \
-        } while (0)
-
 #define GFID_ACCESS_ENTRY_OP_CHECK(loc,err,lbl)    do {                 \
                 /* need to check if the lookup is on virtual dir */     \
                 if ((loc->name && !strcmp (GF_GFID_DIR, loc->name)) &&  \