gfapi: Duplicate the buffer sent in setxattr calls

Issue:
The caller of glfs_setxattr sends a buffer to set as the value.
We create a dict in which the pointer to the value is set.
Underlying layers like md-cache take a ref on this dict to store
the value for a longer time. But the moment setxattr is complete,
the caller of glfs_setxattr can free the value memory.

Solution:
memcpy the setxattr value to the gluster buffer.

> Reviewed-on: https://review.gluster.org/17967
> Reviewed-by: soumya k <skoduri@redhat.com>
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
> Reviewed-by: Jeff Darcy <jeff@pl.atyp.us>
> (cherry picked from commit e11296f8e52b7e3b13d21b41d4fa34baea878edf)

Change-Id: I58753fe702e8b7d0f6c4f058714c65d0ad5d7a0a
BUG: 1479655
Signed-off-by: Poornima G <pgurusid@redhat.com>
Reviewed-on: https://review.gluster.org/18001
Smoke: Gluster Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>

7 files changed, 34 insertions, 12 deletions

diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c
index e8d4f9af18f..c8ddeea196e 100644
--- a/api/src/glfs-fops.c
+++ b/api/src/glfs-fops.c
@@ -3571,6 +3571,7 @@ glfs_setxattr_common (struct glfs *fs, const char *path, const char *name,
 	struct iatt      iatt = {0, };
 	dict_t          *xattr = NULL;
 	int              reval = 0;
+        void            *value_cp = NULL;
 
         DECLARE_OLD_THIS;
         __GLFS_ENTRY_VALIDATE_FS (fs, invalid_fs);
@@ -3605,8 +3606,13 @@ retry:
 	if (ret)
 		goto out;
 
-	xattr = dict_for_key_value (name, value, size);
+        value_cp = gf_memdup (value, size);
+        GF_CHECK_ALLOC_AND_LOG (subvol->name, value_cp, ret, "Failed to"
+                                " duplicate setxattr value", out);
+
+	xattr = dict_for_key_value (name, value_cp, size, _gf_false);
 	if (!xattr) {
+                GF_FREE (value_cp);
 		ret = -1;
 		errno = ENOMEM;
 		goto out;
@@ -3615,8 +3621,6 @@ retry:
 	ret = syncop_setxattr (subvol, &loc, xattr, flags, NULL, NULL);
         DECODE_SYNCOP_ERR (ret);
 
-	ESTALE_RETRY (ret, errno, reval, &loc, retry);
-
 out:
 	loc_wipe (&loc);
 	if (xattr)
@@ -3659,6 +3663,7 @@ pub_glfs_fsetxattr (struct glfs_fd *glfd, const char *name, const void *value,
 	xlator_t        *subvol = NULL;
 	dict_t          *xattr = NULL;
 	fd_t            *fd = NULL;
+        void            *value_cp = NULL;
 
         DECLARE_OLD_THIS;
 	__GLFS_ENTRY_VALIDATE_FD (glfd, invalid_fs);
@@ -3691,8 +3696,13 @@ pub_glfs_fsetxattr (struct glfs_fd *glfd, const char *name, const void *value,
 		goto out;
 	}
 
-	xattr = dict_for_key_value (name, value, size);
+        value_cp = gf_memdup (value, size);
+        GF_CHECK_ALLOC_AND_LOG (subvol->name, value_cp, ret, "Failed to"
+                                " duplicate setxattr value", out);
+
+	xattr = dict_for_key_value (name, value_cp, size, _gf_false);
 	if (!xattr) {
+                GF_FREE (value_cp);
 		ret = -1;
 		errno = ENOMEM;
 		goto out;
diff --git a/api/src/glfs-handleops.c b/api/src/glfs-handleops.c
index dbffa9e26bf..4180f5cf777 100644
--- a/api/src/glfs-handleops.c
+++ b/api/src/glfs-handleops.c
@@ -481,6 +481,7 @@ pub_glfs_h_setxattrs (struct glfs *fs, struct glfs_object *object,
         inode_t         *inode = NULL;
         loc_t            loc = {0, };
         dict_t          *xattr = NULL;
+        void            *value_cp = NULL;
 
         /* validate in args */
         if ((fs == NULL) || (object == NULL) ||
@@ -517,8 +518,13 @@ pub_glfs_h_setxattrs (struct glfs *fs, struct glfs_object *object,
                 goto out;
         }
 
-        xattr = dict_for_key_value (name, value, size);
+        value_cp = gf_memdup (value, size);
+        GF_CHECK_ALLOC_AND_LOG (subvol->name, value_cp, ret, "Failed to"
+                                " duplicate setxattr value", out);
+
+        xattr = dict_for_key_value (name, value_cp, size, _gf_false);
         if (!xattr) {
+                GF_FREE (value_cp);
                 ret = -1;
                 errno = ENOMEM;
                 goto out;
diff --git a/api/src/glfs-internal.h b/api/src/glfs-internal.h
index 838b5c748b2..180981830d7 100644
--- a/api/src/glfs-internal.h
+++ b/api/src/glfs-internal.h
@@ -373,7 +373,6 @@ int glfs_loc_touchup (loc_t *loc)
 void glfs_iatt_to_stat (struct glfs *fs, struct iatt *iatt, struct stat *stat);
 int glfs_loc_link (loc_t *loc, struct iatt *iatt);
 int glfs_loc_unlink (loc_t *loc);
-dict_t *dict_for_key_value (const char *name, const char *value, size_t size);
 int glfs_getxattr_process (void *value, size_t size, dict_t *xattr,
 			   const char *name);
 
diff --git a/libglusterfs/src/dict.c b/libglusterfs/src/dict.c
index aa06dcb2a8d..22bf3f99d70 100644
--- a/libglusterfs/src/dict.c
+++ b/libglusterfs/src/dict.c
@@ -3060,7 +3060,8 @@ dict_dump_to_statedump (dict_t *dict, char *dict_name, char *domain)
 }
 
 dict_t *
-dict_for_key_value (const char *name, const char *value, size_t size)
+dict_for_key_value (const char *name, const char *value, size_t size,
+                    gf_boolean_t is_static)
 {
 	dict_t *xattr = NULL;
 	int     ret = 0;
@@ -3069,7 +3070,12 @@ dict_for_key_value (const char *name, const char *value, size_t size)
 	if (!xattr)
 		return NULL;
 
-	ret = dict_set_static_bin (xattr, (char *)name, (void *)value, size);
+        if (is_static)
+                ret = dict_set_static_bin (xattr, (char *)name, (void *)value,
+                                           size);
+        else
+                ret = dict_set_bin (xattr, (char *)name, (void *)value, size);
+
 	if (ret) {
 		dict_destroy (xattr);
 		xattr = NULL;
diff --git a/libglusterfs/src/dict.h b/libglusterfs/src/dict.h
index b1ba3c20234..93ffa3e8cc7 100644
--- a/libglusterfs/src/dict.h
+++ b/libglusterfs/src/dict.h
@@ -256,7 +256,8 @@ gf_boolean_t
 dict_match_everything (dict_t *d, char *k, data_t *v, void *data);
 
 dict_t *
-dict_for_key_value (const char *name, const char *value, size_t size);
+dict_for_key_value (const char *name, const char *value, size_t size,
+                    gf_boolean_t is_static);
 
 gf_boolean_t
 are_dicts_equal (dict_t *one, dict_t *two,
diff --git a/xlators/features/bit-rot/src/bitd/bit-rot.c b/xlators/features/bit-rot/src/bitd/bit-rot.c
index 3c42cb2a1f6..c5b4037ee98 100644
--- a/xlators/features/bit-rot/src/bitd/bit-rot.c
+++ b/xlators/features/bit-rot/src/bitd/bit-rot.c
@@ -421,7 +421,7 @@ br_object_read_sign (inode_t *linked_inode, fd_t *fd, br_object_t *object,
 
         xattr = dict_for_key_value
                 (GLUSTERFS_SET_OBJECT_SIGNATURE,
-                 (void *)sign, signature_size (SHA256_DIGEST_LENGTH));
+                 (void *)sign, signature_size (SHA256_DIGEST_LENGTH), _gf_true);
 
         if (!xattr) {
                 gf_msg (this->name, GF_LOG_ERROR, 0, BRB_MSG_SET_SIGN_FAILED,
diff --git a/xlators/features/upcall/src/upcall.c b/xlators/features/upcall/src/upcall.c
index e20a2e87093..3e1d307260a 100644
--- a/xlators/features/upcall/src/upcall.c
+++ b/xlators/features/upcall/src/upcall.c
@@ -1876,7 +1876,7 @@ up_fremovexattr (call_frame_t *frame, xlator_t *this, fd_t *fd,
 
         EXIT_IF_UPCALL_OFF (this, out);
 
-        xattr = dict_for_key_value (name, "", 1);
+        xattr = dict_for_key_value (name, "", 1, _gf_true);
         if (!xattr) {
                 op_errno = ENOMEM;
                 goto err;
@@ -1964,7 +1964,7 @@ up_removexattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
 
         EXIT_IF_UPCALL_OFF (this, out);
 
-        xattr = dict_for_key_value (name, "", 1);
+        xattr = dict_for_key_value (name, "", 1, _gf_true);
         if (!xattr) {
                 op_errno = ENOMEM;
                 goto err;