summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNiels de Vos <ndevos@redhat.com>2015-07-12 12:29:12 +0200
committerNiels de Vos <ndevos@redhat.com>2015-07-24 08:09:08 -0700
commit2d2d1fbb201ea81a54d9c9aef28345cb259eb141 (patch)
treeed1af323f987a13b21caa4206d1bc2d569b9ae76
parent8a4f0de2a6dc5d1a9cbc54bbe9aa25d7e38d4ae7 (diff)
dict: dict_set_bin() should never free the pointer on error
dict_set_bin() is handling the pointer that it passed inconsistently. Depending on the errors that can occur, the pointer passed to the dict can be free'd, but there is no guarantee. It is cleaner to have the caller free the pointer that allocated it and dict_set_bin() returned an error. When dict_set_bin() returned success, the given pointer will be free'd when dict_unref() calls data_destroy(). Many callers of dict_set_bin() already take care of free'ing the pointer on error. The ones that did not, are corrected with this change too. Change-Id: I39a4f7ebc0cae6d403baba99307d7ce408f25966 BUG: 1242280 Signed-off-by: Niels de Vos <ndevos@redhat.com> Reviewed-on: http://review.gluster.org/11638 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: jiffin tony Thottan <jthottan@redhat.com> Reviewed-by: Raghavendra G <rgowdapp@redhat.com> Tested-by: NetBSD Build System <jenkins@build.gluster.org>
-rw-r--r--libglusterfs/src/dict.c5
-rw-r--r--xlators/cluster/afr/src/afr-self-heal-common.c8
-rw-r--r--xlators/cluster/dht/src/dht-rename.c1
-rw-r--r--xlators/cluster/dht/src/dht-selfheal.c3
-rw-r--r--xlators/cluster/ec/src/ec-helpers.c20
-rw-r--r--xlators/features/bit-rot/src/stub/bit-rot-stub.c4
-rw-r--r--xlators/features/marker/src/marker-quota.c22
-rw-r--r--xlators/features/marker/src/marker.c4
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-locks.c3
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-mgmt.c2
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-op-sm.c2
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-replace-brick.c1
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-utils.c4
-rw-r--r--xlators/storage/posix/src/posix.c31
-rw-r--r--xlators/system/posix-acl/src/posix-acl.c2
15 files changed, 74 insertions, 38 deletions
diff --git a/libglusterfs/src/dict.c b/libglusterfs/src/dict.c
index d191164..c58ce80 100644
--- a/libglusterfs/src/dict.c
+++ b/libglusterfs/src/dict.c
@@ -2286,8 +2286,11 @@ dict_set_bin_common (dict_t *this, char *key, void *ptr, size_t size,
data->is_static = is_static;
ret = dict_set (this, key, data);
- if (ret < 0)
+ if (ret < 0) {
+ /* don't free data->data, let callers handle it */
+ data->data = NULL;
data_destroy (data);
+ }
err:
return ret;
diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c
index edacdde..a7633c9 100644
--- a/xlators/cluster/afr/src/afr-self-heal-common.c
+++ b/xlators/cluster/afr/src/afr-self-heal-common.c
@@ -81,8 +81,10 @@ afr_selfheal_output_xattr (xlator_t *this, afr_transaction_type type,
raw[idx] = hton32 (output_dirty[subvol]);
ret = dict_set_bin (xattr, AFR_DIRTY, raw,
sizeof(int) * AFR_NUM_CHANGE_LOGS);
- if (ret)
+ if (ret) {
+ GF_FREE (raw);
goto err;
+ }
/* clear/set pending */
for (j = 0; j < priv->child_count; j++) {
@@ -95,8 +97,10 @@ afr_selfheal_output_xattr (xlator_t *this, afr_transaction_type type,
ret = dict_set_bin (xattr, priv->pending_key[j],
raw, sizeof(int) * AFR_NUM_CHANGE_LOGS);
- if (ret)
+ if (ret) {
+ GF_FREE (raw);
goto err;
+ }
}
return xattr;
diff --git a/xlators/cluster/dht/src/dht-rename.c b/xlators/cluster/dht/src/dht-rename.c
index b6ed2ae..bde0ce8 100644
--- a/xlators/cluster/dht/src/dht-rename.c
+++ b/xlators/cluster/dht/src/dht-rename.c
@@ -359,6 +359,7 @@ dht_rename_track_for_changelog (xlator_t *this, dict_t *xattr,
"Failed to set dictionary value: key = %s,"
" path = %s", DHT_CHANGELOG_RENAME_OP_KEY,
oldloc->name);
+ GF_FREE (info);
}
return ret;
}
diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c
index a7fcc9c..ca0507b 100644
--- a/xlators/cluster/dht/src/dht-selfheal.c
+++ b/xlators/cluster/dht/src/dht-selfheal.c
@@ -680,6 +680,7 @@ dht_selfheal_dir_xattr_persubvol (call_frame_t *frame, loc_t *loc,
"Directory self heal xattr failed:"
"%s: (subvol %s) Failed to set xattr dictionary,"
" gfid = %s", loc->path, subvol->name, gfid);
+ GF_FREE (disk_layout);
goto err;
}
disk_layout = NULL;
@@ -2126,6 +2127,8 @@ dht_update_commit_hash_for_layout_resume (call_frame_t *frame, void *cookie,
" dictionary,", local->loc.path,
conf->local_subvols[i]->name);
+ GF_FREE (disk_layout);
+
goto err;
}
disk_layout = NULL;
diff --git a/xlators/cluster/ec/src/ec-helpers.c b/xlators/cluster/ec/src/ec-helpers.c
index 372633d..aec0831 100644
--- a/xlators/cluster/ec/src/ec-helpers.c
+++ b/xlators/cluster/ec/src/ec-helpers.c
@@ -161,6 +161,7 @@ size_t ec_iov_copy_to(void * dst, struct iovec * vector, int32_t count,
int32_t ec_dict_set_array(dict_t *dict, char *key, uint64_t value[],
int32_t size)
{
+ int ret = -1;
uint64_t *ptr = NULL;
int32_t vindex;
if (value == NULL)
@@ -172,7 +173,10 @@ int32_t ec_dict_set_array(dict_t *dict, char *key, uint64_t value[],
for (vindex = 0; vindex < size; vindex++) {
ptr[vindex] = hton64(value[vindex]);
}
- return dict_set_bin(dict, key, ptr, sizeof(uint64_t) * size);
+ ret = dict_set_bin(dict, key, ptr, sizeof(uint64_t) * size);
+ if (ret)
+ GF_FREE (ptr);
+ return ret;
}
@@ -214,6 +218,7 @@ int32_t ec_dict_del_array(dict_t *dict, char *key, uint64_t value[],
int32_t ec_dict_set_number(dict_t * dict, char * key, uint64_t value)
{
+ int ret = -1;
uint64_t * ptr;
ptr = GF_MALLOC(sizeof(value), gf_common_mt_char);
@@ -224,7 +229,11 @@ int32_t ec_dict_set_number(dict_t * dict, char * key, uint64_t value)
*ptr = hton64(value);
- return dict_set_bin(dict, key, ptr, sizeof(value));
+ ret = dict_set_bin(dict, key, ptr, sizeof(value));
+ if (ret)
+ GF_FREE (ptr);
+
+ return ret;
}
int32_t ec_dict_del_number(dict_t * dict, char * key, uint64_t * value)
@@ -247,6 +256,7 @@ int32_t ec_dict_del_number(dict_t * dict, char * key, uint64_t * value)
int32_t ec_dict_set_config(dict_t * dict, char * key, ec_config_t * config)
{
+ int ret = -1;
uint64_t * ptr, data;
if (config->version > EC_CONFIG_VERSION)
@@ -274,7 +284,11 @@ int32_t ec_dict_set_config(dict_t * dict, char * key, ec_config_t * config)
*ptr = hton64(data);
- return dict_set_bin(dict, key, ptr, sizeof(uint64_t));
+ ret = dict_set_bin(dict, key, ptr, sizeof(uint64_t));
+ if (ret)
+ GF_FREE (ptr);
+
+ return ret;
}
int32_t ec_dict_del_config(dict_t * dict, char * key, ec_config_t * config)
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub.c b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
index 2efb63e..9fa16c3 100644
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub.c
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
@@ -1345,8 +1345,10 @@ br_stub_getxattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
op_errno = EINVAL;
ret = dict_set_bin (xattr, GLUSTERFS_GET_OBJECT_SIGNATURE,
(void *)sign, totallen);
- if (ret < 0)
+ if (ret < 0) {
+ GF_FREE (sign);
goto delkeys;
+ }
op_errno = 0;
op_ret = totallen;
diff --git a/xlators/features/marker/src/marker-quota.c b/xlators/features/marker/src/marker-quota.c
index 52b930f..8479852 100644
--- a/xlators/features/marker/src/marker-quota.c
+++ b/xlators/features/marker/src/marker-quota.c
@@ -401,6 +401,12 @@ mq_update_size_xattr (call_frame_t *frame, void *cookie, xlator_t *this,
goto err;
}
+ new_dict = dict_new ();
+ if (!new_dict) {
+ errno = ENOMEM;
+ goto err;
+ }
+
QUOTA_ALLOC_OR_GOTO (delta, int64_t, ret, err);
*delta = hton64 (local->sum - ntoh64 (*size));
@@ -410,15 +416,11 @@ mq_update_size_xattr (call_frame_t *frame, void *cookie, xlator_t *this,
" path = %s diff = %"PRIu64, local->sum, ntoh64 (*size),
local->loc.path, ntoh64 (*delta));
- new_dict = dict_new ();
- if (!new_dict) {
- errno = ENOMEM;
- goto err;
- }
-
ret = dict_set_bin (new_dict, QUOTA_SIZE_KEY, delta, 8);
- if (ret)
+ if (ret) {
+ GF_FREE (delta);
goto err;
+ }
if (gf_uuid_is_null (local->loc.gfid))
gf_uuid_copy (local->loc.gfid, buf->ia_gfid);
@@ -1649,6 +1651,7 @@ mq_update_parent_size (call_frame_t *frame,
ret = dict_set_bin (newdict, QUOTA_SIZE_KEY, size, 8);
if (ret < 0) {
+ GF_FREE (size);
op_errno = -ret;
goto err;
}
@@ -1776,6 +1779,7 @@ unlock:
ret = dict_set_bin (newdict, contri_key, delta, 8);
if (ret < 0) {
+ GF_FREE (delta);
op_errno = -ret;
ret = -1;
goto err;
@@ -3998,8 +4002,10 @@ mq_reduce_parent_size_xattr (call_frame_t *frame, void *cookie, xlator_t *this,
*size = hton64 (-local->size);
ret = dict_set_bin (dict, QUOTA_SIZE_KEY, size, 8);
- if (ret < 0)
+ if (ret < 0) {
+ GF_FREE (size);
goto err;
+ }
gf_uuid_copy (local->parent_loc.gfid,
local->parent_loc.inode->gfid);
diff --git a/xlators/features/marker/src/marker.c b/xlators/features/marker/src/marker.c
index fbe640f..8d39bec 100644
--- a/xlators/features/marker/src/marker.c
+++ b/xlators/features/marker/src/marker.c
@@ -260,9 +260,11 @@ marker_getxattr_stampfile_cbk (call_frame_t *frame, xlator_t *this,
ret = dict_set_bin (dict, (char *)name, vol_mark,
sizeof (struct volume_mark));
- if (ret)
+ if (ret) {
+ GF_FREE (vol_mark);
gf_log (this->name, GF_LOG_WARNING, "failed to set key %s",
name);
+ }
STACK_UNWIND_STRICT (getxattr, frame, 0, 0, dict, xdata);
diff --git a/xlators/mgmt/glusterd/src/glusterd-locks.c b/xlators/mgmt/glusterd/src/glusterd-locks.c
index e18bd23..146092d 100644
--- a/xlators/mgmt/glusterd/src/glusterd-locks.c
+++ b/xlators/mgmt/glusterd/src/glusterd-locks.c
@@ -590,8 +590,7 @@ glusterd_mgmt_v3_lock (const char *name, uuid_t uuid, uint32_t *op_errno,
gf_msg (this->name, GF_LOG_ERROR, 0,
GD_MSG_DICT_SET_FAILED,
"Unable to set lock owner in mgmt_v3 lock");
- if (lock_obj)
- GF_FREE (lock_obj);
+ GF_FREE (lock_obj);
goto out;
}
diff --git a/xlators/mgmt/glusterd/src/glusterd-mgmt.c b/xlators/mgmt/glusterd/src/glusterd-mgmt.c
index 326d868..2afa696 100644
--- a/xlators/mgmt/glusterd/src/glusterd-mgmt.c
+++ b/xlators/mgmt/glusterd/src/glusterd-mgmt.c
@@ -1817,6 +1817,7 @@ glusterd_mgmt_v3_initiate_all_phases (rpcsvc_request_t *req, glusterd_op_t op,
gf_msg (this->name, GF_LOG_ERROR, 0,
GD_MSG_DICT_SET_FAILED,
"Failed to set originator_uuid.");
+ GF_FREE (originator_uuid);
goto out;
}
@@ -2051,6 +2052,7 @@ glusterd_mgmt_v3_initiate_snap_phases (rpcsvc_request_t *req, glusterd_op_t op,
gf_msg (this->name, GF_LOG_ERROR, 0,
GD_MSG_DICT_SET_FAILED,
"Failed to set originator_uuid.");
+ GF_FREE (originator_uuid);
goto out;
}
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
index 07a407b..68cffef 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
@@ -5142,6 +5142,7 @@ glusterd_op_ac_stage_op (glusterd_op_sm_event_t *event, void *ctx)
gf_msg (this->name, GF_LOG_ERROR, 0,
GD_MSG_DICT_SET_FAILED,
"Failed to set transaction id.");
+ GF_FREE (txn_id);
goto out;
}
@@ -5256,6 +5257,7 @@ glusterd_op_ac_commit_op (glusterd_op_sm_event_t *event, void *ctx)
gf_msg (this->name, GF_LOG_ERROR, 0,
GD_MSG_DICT_SET_FAILED,
"Failed to set transaction id.");
+ GF_FREE (txn_id);
goto out;
}
diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
index 4ba7d8d..70ee736 100644
--- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
+++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
@@ -868,6 +868,7 @@ glusterd_mgmt_v3_initiate_replace_brick_cmd_phases (rpcsvc_request_t *req,
gf_msg (this->name, GF_LOG_ERROR, 0,
GD_MSG_DICT_SET_FAILED,
"Failed to set originator_uuid.");
+ GF_FREE (originator_uuid);
goto out;
}
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 501e9de..ffc6160 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -7308,8 +7308,10 @@ glusterd_append_status_dicts (dict_t *dst, dict_t *src)
snprintf (sts_val_name, sizeof(sts_val_name), "status_value%d", i + dst_count);
ret = dict_set_bin (dst, sts_val_name, dst_sts_val, sizeof(gf_gsync_status_t));
- if (ret)
+ if (ret) {
+ GF_FREE (dst_sts_val);
goto out;
+ }
}
ret = dict_set_int32 (dst, "gsync-count", dst_count+src_count);
diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c
index 1089b7f..7589e8c 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -3365,16 +3365,13 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
ret = dict_set_bin (xattr, POSIX_ACL_ACCESS_XATTR,
acl_xattr, acl_size);
- if (ret)
+ if (ret) {
gf_msg (this->name, GF_LOG_WARNING, 0,
P_MSG_SET_XDATA_FAIL, "failed to set"
"xdata for acl");
-
- /*
- * dict_unref() will call GF_FREE() indirectly, so to avoid
- * double freeing acl_xattr in out, just set it as NULL here
- */
- acl_xattr = NULL;
+ GF_FREE (acl_xattr);
+ goto out;
+ }
}
if (dict_get (dict, GF_POSIX_ACL_DEFAULT)) {
@@ -3406,16 +3403,13 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
ret = dict_set_bin (xattr, POSIX_ACL_DEFAULT_XATTR,
acl_xattr, acl_size);
- if (ret)
+ if (ret) {
gf_msg (this->name, GF_LOG_WARNING, 0,
P_MSG_SET_XDATA_FAIL, "failed to set"
"xdata for acl");
-
- /*
- * dict_unref() will call GF_FREE() indirectly, so to avoid
- * double freeing acl_xattr in out, just set it as NULL here
- */
- acl_xattr = NULL;
+ GF_FREE (acl_xattr);
+ goto out;
+ }
}
out:
SET_TO_OLD_FS_ID ();
@@ -3423,8 +3417,7 @@ out:
STACK_UNWIND_STRICT (setxattr, frame, op_ret, op_errno, xattr);
if (xattr)
- dict_unref(xattr);
- GF_FREE (acl_xattr);
+ dict_unref (xattr);
return 0;
}
@@ -4963,9 +4956,8 @@ unlock:
op_ret = -1;
goto out;
} else {
- size = dict_set_bin (d, k, array, v->len);
-
- if (size != 0) {
+ op_ret = dict_set_bin (d, k, array, v->len);
+ if (op_ret) {
if (filler->real_path)
gf_msg_debug (this->name, 0,
"dict_set_bin failed (path=%s): "
@@ -4980,6 +4972,7 @@ unlock:
op_ret = -1;
op_errno = EINVAL;
+ GF_FREE (array);
goto out;
}
array = NULL;
diff --git a/xlators/system/posix-acl/src/posix-acl.c b/xlators/system/posix-acl/src/posix-acl.c
index b1d3df3..f4527c3 100644
--- a/xlators/system/posix-acl/src/posix-acl.c
+++ b/xlators/system/posix-acl/src/posix-acl.c
@@ -659,6 +659,7 @@ posix_acl_inherit (xlator_t *this, loc_t *loc, dict_t *params, mode_t mode,
size_access);
if (ret) {
gf_log (this->name, GF_LOG_ERROR, "out of memory");
+ GF_FREE (xattr_access);
ret = -1;
goto out;
}
@@ -682,6 +683,7 @@ posix_acl_inherit (xlator_t *this, loc_t *loc, dict_t *params, mode_t mode,
size_default);
if (ret) {
gf_log (this->name, GF_LOG_ERROR, "out of memory");
+ GF_FREE (xattr_default);
ret = -1;
goto out;
}