From 3fe535afa61fa06c066e511c3c6e269902924296 Mon Sep 17 00:00:00 2001 From: Poornima G Date: Thu, 3 Aug 2017 17:43:22 +0530 Subject: 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 > Smoke: Gluster Build System > CentOS-regression: Gluster Build System > Reviewed-by: Jeff Darcy > (cherry picked from commit e11296f8e52b7e3b13d21b41d4fa34baea878edf) Change-Id: I58753fe702e8b7d0f6c4f058714c65d0ad5d7a0a BUG: 1479656 Signed-off-by: Poornima G Reviewed-on: https://review.gluster.org/18002 Smoke: Gluster Build System CentOS-regression: Gluster Build System Reviewed-by: Shyamsundar Ranganathan --- 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 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 14268f4b3b2..5dea6dccd74 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 839b42685e8..1aa873e77bd 100644 --- a/libglusterfs/src/dict.c +++ b/libglusterfs/src/dict.c @@ -3018,7 +3018,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; @@ -3027,7 +3028,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 bef58e102cc..72198d43d50 100644 --- a/libglusterfs/src/dict.h +++ b/libglusterfs/src/dict.h @@ -254,7 +254,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 c591db53ad4..7f735b4b768 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; -- cgit