pump: cleanup potential dict related memory corruption.v3.1.5qa2

Signed-off-by: Krishnan Parthasarathi <kp@gluster.com>
Signed-off-by: Anand Avati <avati@gluster.com>

BUG: 2489 (GlusterFS crashing with replace-brick)
URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=2489

5 files changed, 103 insertions, 40 deletions

diff --git a/libglusterfs/src/mem-types.h b/libglusterfs/src/mem-types.h
index 3a55bdc7c77..5fb1f80955e 100644
--- a/libglusterfs/src/mem-types.h
+++ b/libglusterfs/src/mem-types.h
@@ -98,6 +98,7 @@ enum gf_common_mem_types_ {
         gf_common_mt_sge                =       73,
         gf_common_mt_rpcclnt_cb_program_t =     74,
         gf_common_mt_libxl_marker_local =       75,
-        gf_common_mt_end                =       76
+        gf_common_mt_int32_t            =       76,
+        gf_common_mt_end                =       77
 };
 #endif
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index 868dab736b9..5c8a8105d30 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -63,13 +63,23 @@
 int32_t
 afr_set_dict_gfid (dict_t *dict, uuid_t gfid)
 {
-        int ret = 0;
+        int     ret       = 0;
+        uuid_t *pgfid     = NULL;
 
         GF_ASSERT (gfid);
+        pgfid = GF_CALLOC (1, sizeof (uuid_t), gf_common_mt_char);
+        if (!pgfid) {
+                ret = -1;
+                goto out;
+        }
+        uuid_copy (*pgfid, gfid);
 
-        ret = dict_set_static_bin (dict, "gfid-req", gfid, 16);
+        ret = dict_set_dynptr (dict, "gfid-req", pgfid, 16);
         if (ret)
                 gf_log (THIS->name, GF_LOG_DEBUG, "gfid set failed");
+out:
+        if (ret && pgfid)
+                GF_FREE (pgfid);
 
         return ret;
 }
diff --git a/xlators/cluster/afr/src/afr-self-heal-data.c b/xlators/cluster/afr/src/afr-self-heal-data.c
index 5adaba1c974..d4516c4ce1e 100644
--- a/xlators/cluster/afr/src/afr-self-heal-data.c
+++ b/xlators/cluster/afr/src/afr-self-heal-data.c
@@ -859,12 +859,11 @@ afr_sh_data_fxattrop_cbk (call_frame_t *frame, void *cookie,
 int
 afr_sh_data_fxattrop (call_frame_t *frame, xlator_t *this)
 {
-	afr_self_heal_t *sh    = NULL;
-	afr_local_t     *local = NULL;
-	afr_private_t   *priv  = NULL;
-	dict_t          *xattr_req = NULL;
-
-        int32_t zero_pending[3] = {0, 0, 0};
+        afr_self_heal_t *sh           = NULL;
+        afr_local_t     *local        = NULL;
+        afr_private_t   *priv         = NULL;
+        dict_t          *xattr_req    = NULL;
+        int32_t         *zero_pending = NULL;
 
 	int call_count = 0;
 	int i = 0;
@@ -880,14 +879,21 @@ afr_sh_data_fxattrop (call_frame_t *frame, xlator_t *this)
 	local->call_count = call_count;
 
 	xattr_req = dict_new();
-	if (xattr_req) {
-                for (i = 0; i < priv->child_count; i++) {
-                        ret = dict_set_static_bin (xattr_req, priv->pending_key[i],
-                                                   zero_pending, 3 * sizeof(int32_t));
-                        if (ret < 0)
-				gf_log (this->name, GF_LOG_WARNING,
-					"Unable to set dict value");
+        if (!xattr_req) {
+                ret = -1;
+                goto out;
+        }
+        for (i = 0; i < priv->child_count; i++) {
+                zero_pending = GF_CALLOC (3, sizeof (int32_t), gf_common_mt_int32_t);
+                if (!zero_pending) {
+                        ret = -1;
+                        goto out;
                 }
+                ret = dict_set_dynptr (xattr_req, priv->pending_key[i],
+                                       zero_pending, 3 * sizeof(int32_t));
+                if (ret < 0)
+                        gf_log (this->name, GF_LOG_WARNING,
+                                "Unable to set dict value");
         }
 
 	for (i = 0; i < priv->child_count; i++) {
@@ -904,8 +910,14 @@ afr_sh_data_fxattrop (call_frame_t *frame, xlator_t *this)
 		}
 	}
 
+out:
 	if (xattr_req)
 		dict_unref (xattr_req);
+        if (ret) {
+                GF_FREE (zero_pending)
+                sh->op_failed = -1;
+                afr_sh_data_done (frame, this);
+        }
 
 	return 0;
 }
diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c
index a4077ddb0f9..0c9fca49572 100644
--- a/xlators/cluster/afr/src/afr-self-heal-entry.c
+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c
@@ -1040,14 +1040,14 @@ afr_sh_entry_impunge_newfile_cbk (call_frame_t *impunge_frame, void *cookie,
                                   struct iatt *preparent,
                                   struct iatt *postparent)
 {
-	int              call_count = 0;
-	afr_private_t   *priv = NULL;
-	afr_local_t     *impunge_local = NULL;
-	afr_self_heal_t *impunge_sh = NULL;
-	call_frame_t    *frame = NULL;
-	int              active_src = 0;
-	int              child_index = 0;
-        int              pending_array[3] = {0, };
+        int              call_count = 0;
+        afr_private_t   *priv = NULL;
+        afr_local_t     *impunge_local = NULL;
+        afr_self_heal_t *impunge_sh = NULL;
+        call_frame_t    *frame = NULL;
+        int              active_src = 0;
+        int              child_index = 0;
+        int32_t         *pending_array = NULL;
         dict_t          *xattr = NULL;
         int              ret = 0;
         int              idx = 0;
@@ -1080,8 +1080,16 @@ afr_sh_entry_impunge_newfile_cbk (call_frame_t *impunge_frame, void *cookie,
 
 	inode->ia_type = stbuf->ia_type;
 
-        xattr = get_new_dict ();
-        dict_ref (xattr);
+        xattr = dict_new ();
+        if (!xattr) {
+                ret = -1;
+                goto out;
+        }
+        pending_array = GF_CALLOC (3, sizeof (int32_t), gf_common_mt_int32_t);
+        if (!pending_array) {
+                ret = -1;
+                goto out;
+        }
 
         idx = afr_index_for_transaction_type (AFR_METADATA_TRANSACTION);
         pending_array[idx] = hton32 (1);
@@ -1101,8 +1109,11 @@ afr_sh_entry_impunge_newfile_cbk (call_frame_t *impunge_frame, void *cookie,
         parentbuf     = impunge_sh->parentbuf;
         setattr_frame = copy_frame (impunge_frame);
 
-        parent_loc = GF_CALLOC (1, sizeof (*parent_loc),
-                                gf_afr_mt_loc_t);
+        parent_loc = GF_CALLOC (1, sizeof (*parent_loc), gf_afr_mt_loc_t);
+        if (!parent_loc) {
+                ret = -1;
+                goto out;
+        }
         afr_build_parent_loc (parent_loc, &impunge_local->loc);
 
 	STACK_WIND_COOKIE (impunge_frame, afr_sh_entry_impunge_xattrop_cbk,
@@ -1122,6 +1133,12 @@ afr_sh_entry_impunge_newfile_cbk (call_frame_t *impunge_frame, void *cookie,
 	return 0;
 
 out:
+        if (ret) {
+                if (xattr)
+                        dict_unref (xattr);
+                if (pending_array)
+                        GF_FREE (pending_array);
+        }
 	LOCK (&impunge_frame->lock);
 	{
 		call_count = --impunge_local->call_count;
@@ -1861,7 +1878,7 @@ afr_sh_entry_impunge_readdir_cbk (call_frame_t *frame, void *cookie,
 
 	return 0;
 }
-				  
+
 
 int
 afr_sh_entry_impunge_subvol (call_frame_t *frame, xlator_t *this,
diff --git a/xlators/cluster/afr/src/pump.c b/xlators/cluster/afr/src/pump.c
index 10b9b720316..a7df543b46c 100644
--- a/xlators/cluster/afr/src/pump.c
+++ b/xlators/cluster/afr/src/pump.c
@@ -860,7 +860,8 @@ pump_initiate_sink_connect (call_frame_t *frame, xlator_t *this)
         afr_local_t   *local     = NULL;
         afr_private_t *priv      = NULL;
         dict_t        *dict      = NULL;
-        char          *dst_brick = NULL;
+        data_t        *data      = NULL;
+        char          *clnt_cmd  = NULL;
         loc_t loc;
 
         int ret = 0;
@@ -872,8 +873,9 @@ pump_initiate_sink_connect (call_frame_t *frame, xlator_t *this)
 
         build_root_loc (priv->root_inode, &loc);
 
-        ret = dict_get_str (local->dict, PUMP_CMD_START, &dst_brick);
-        if (ret < 0) {
+        data = data_ref (dict_get (local->dict, PUMP_CMD_START));
+        if (!data) {
+                ret = -1;
                 gf_log (this->name, GF_LOG_ERROR,
                         "Could not get destination brick value");
                 goto out;
@@ -887,11 +889,16 @@ pump_initiate_sink_connect (call_frame_t *frame, xlator_t *this)
                 goto out;
         }
 
-        GF_ASSERT (dst_brick);
+        clnt_cmd = GF_CALLOC (1, data->len+1, gf_common_mt_char);
+        if (!clnt_cmd) {
+                ret = -1;
+                goto out;
+        }
+        memcpy (clnt_cmd, data->data, data->len);
         gf_log (this->name, GF_LOG_DEBUG,
-                "Got destination brick as %s", dst_brick);
+                "Got destination brick as %s", clnt_cmd);
 
-        ret = dict_set_str (dict, CLIENT_CMD_CONNECT, dst_brick);
+        ret = dict_set_dynstr (dict, CLIENT_CMD_CONNECT, clnt_cmd);
         if (ret < 0) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "Could not inititiate destination brick "
@@ -909,8 +916,14 @@ pump_initiate_sink_connect (call_frame_t *frame, xlator_t *this)
 
         ret = 0;
 
-        dict_unref (dict);
 out:
+        if (dict)
+                dict_unref (dict);
+        if (data)
+                data_unref (data);
+        if (ret)
+                GF_FREE (clnt_cmd);
+
         return ret;
 }
 
@@ -1019,7 +1032,6 @@ pump_execute_status (call_frame_t *frame, xlator_t *this)
         if (!dict_str) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "Out of memory");
-                op_ret = -1;
                 op_errno = ENOMEM;
                 goto out;
         }
@@ -1033,21 +1045,32 @@ pump_execute_status (call_frame_t *frame, xlator_t *this)
         }
 
         dict = dict_new ();
+        if (!dict) {
+                ret = -1;
+                op_errno = ENOMEM;
+                goto out;
+        }
 
-        ret = dict_set_str (dict, PUMP_CMD_STATUS, dict_str);
+        ret = dict_set_dynstr (dict, PUMP_CMD_STATUS, dict_str);
         if (ret < 0) {
+                op_errno = -ret;
                 gf_log (this->name, GF_LOG_DEBUG,
                         "dict_set_str returned negative value");
+                goto out;
         }
 
         op_ret = 0;
 
 out:
+        if (ret)
+                op_ret = -1;
 
         AFR_STACK_UNWIND (getxattr, frame, op_ret, op_errno, dict);
 
-        dict_unref (dict);
-        GF_FREE (dict_str);
+        if (dict)
+                dict_unref (dict);
+        if (ret)
+                GF_FREE (dict_str);
 
         return 0;
 }