summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKrishnan Parthasarathi <kp@gluster.com>2011-06-10 04:42:28 +0000
committerAnand Avati <avati@gluster.com>2011-06-10 03:53:58 -0700
commit25da481bc5b06d671e41e5a70b2c145777154bf1 (patch)
treec58240c266f711778181d96e270f9ea6a053e145
parentfb42a67e1eea17b3dc116d26ea92b93f740b28c3 (diff)
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
-rw-r--r--libglusterfs/src/mem-types.h3
-rw-r--r--xlators/cluster/afr/src/afr-common.c14
-rw-r--r--xlators/cluster/afr/src/afr-self-heal-data.c38
-rw-r--r--xlators/cluster/afr/src/afr-self-heal-entry.c43
-rw-r--r--xlators/cluster/afr/src/pump.c45
5 files changed, 103 insertions, 40 deletions
diff --git a/libglusterfs/src/mem-types.h b/libglusterfs/src/mem-types.h
index 3a55bdc..5fb1f80 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 868dab7..5c8a810 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 5adaba1..d4516c4 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 a4077dd..0c9fca4 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 10b9b72..a7df543 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;
}