From eb266456a714fe5c31444d3bdbfdfa744382ac50 Mon Sep 17 00:00:00 2001

From: Poornima G <pgurusid@redhat.com>

Date: Thu, 3 Aug 2017 17:43:22 +0530

Subject: [PATCH 599/601] 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: 1457713

Signed-off-by: Poornima G <pgurusid@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/114763

Reviewed-by: Atin Mukherjee <amukherj@redhat.com>

---

 api/src/glfs-fops.c                         | 18 ++++++++++++++----

 api/src/glfs-handleops.c                    |  8 +++++++-

 api/src/glfs-internal.h                     |  1 -

 libglusterfs/src/dict.c                     | 10 ++++++++--

 libglusterfs/src/dict.h                     |  3 ++-

 xlators/features/bit-rot/src/bitd/bit-rot.c |  2 +-

 xlators/features/upcall/src/upcall.c        |  4 ++--

 7 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c

index 41d4795..3bba7d6 100644

--- a/api/src/glfs-fops.c

+++ b/api/src/glfs-fops.c

@@ -3572,6 +3572,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);

@@ -3606,8 +3607,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;

@@ -3616,8 +3622,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)

@@ -3660,6 +3664,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);

@@ -3692,8 +3697,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 a464b2e..88f9ab1 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 f78fcfd..3f4ed71 100644

--- a/api/src/glfs-internal.h

+++ b/api/src/glfs-internal.h

@@ -367,7 +367,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 d0c56d5..8881173 100644

--- a/libglusterfs/src/dict.c

+++ b/libglusterfs/src/dict.c

@@ -2992,7 +2992,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;

@@ -3001,7 +3002,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 b0efad7..b5d9f3e 100644

--- a/libglusterfs/src/dict.h

+++ b/libglusterfs/src/dict.h

@@ -257,7 +257,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 d88231b..55191da 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 a9d3c8a..0f1d980 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;

-- 

1.8.3.1