From a8a2d6d3bb161e86464c9bb72b8847fe3ef7699e Mon Sep 17 00:00:00 2001 From: Krutika Dhananjay Date: Sat, 3 Dec 2016 09:09:15 +0530 Subject: afr, client: More mem-leak fixes in COMPOUND fop cbk Backport of: http://review.gluster.org/16020 Bugs found and fixed: 1. Use correct subvolume index in pre-op-writev compound cbk 2. Prevent use-after-free of local->compound_args members in compound fops cbk in protocol/client 3. Fix xdata and xattr leaks in client_process_response 4. Fix possible leak of xdata in client_pre_writev() in test mode. 5. Free req->compound_req_array.compound_req_array_val as well after freeing its members 6. Free tmp_rsp->flock.lk_owner.lk_owner_val in LK fop. Change-Id: I40f576b48625d65fcbd7a11181eeff37f9e1e011 BUG: 1402212 Signed-off-by: Krutika Dhananjay Reviewed-on: http://review.gluster.org/16046 Smoke: Gluster Build System NetBSD-regression: NetBSD Build System CentOS-regression: Gluster Build System Reviewed-by: Pranith Kumar Karampuri --- xlators/cluster/afr/src/afr-common.c | 12 ------------ xlators/cluster/afr/src/afr-transaction.c | 25 +++++++++++++++++++++---- xlators/cluster/afr/src/afr.h | 5 +---- 3 files changed, 22 insertions(+), 20 deletions(-) (limited to 'xlators/cluster/afr') diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index e1ed4fc5b44..4838c9cd638 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -5946,18 +5946,6 @@ afr_pack_fop_args (call_frame_t *frame, compound_args_t *args, return NULL; } -void -afr_compound_cleanup (compound_args_t *args, dict_t *xdata, - dict_t *newloc_xdata) -{ - if (args) - compound_args_cleanup (args); - if (xdata) - dict_unref (xdata); - if (newloc_xdata) - dict_unref (newloc_xdata); -} - int afr_fav_child_reset_sink_xattrs_cbk (int ret, call_frame_t *heal_frame, void *opaque) diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c index 3def97c5ae3..f40df5c8ea3 100644 --- a/xlators/cluster/afr/src/afr-transaction.c +++ b/xlators/cluster/afr/src/afr-transaction.c @@ -1284,7 +1284,7 @@ afr_pre_op_writev_cbk (call_frame_t *frame, void *cookie, xlator_t *this, NULL, NULL, NULL); } else { write_args_cbk = &args_cbk->rsp_list[1]; - afr_inode_write_fill (frame, this, (long) i, + afr_inode_write_fill (frame, this, (long) child_index, write_args_cbk->op_ret, write_args_cbk->op_errno, &write_args_cbk->prestat, @@ -1295,6 +1295,8 @@ afr_pre_op_writev_cbk (call_frame_t *frame, void *cookie, xlator_t *this, call_count = afr_frame_return (frame); if (call_count == 0) { + compound_args_cleanup (local->c_args); + local->c_args = NULL; afr_process_post_writev (frame, this); if (!afr_txn_nothing_failed (frame, this)) { /* Don't unwind until post-op is complete */ @@ -1386,6 +1388,8 @@ afr_pre_op_fop_do (call_frame_t *frame, xlator_t *this, dict_t *xattr, */ compound_cbk = afr_pack_fop_args (frame, args, local->op, i); + local->c_args = args; + for (i = 0; i < priv->child_count; i++) { /* Means lock did not succeed on this brick */ if (!local->transaction.pre_op[i]) @@ -1401,7 +1405,10 @@ afr_pre_op_fop_do (call_frame_t *frame, xlator_t *this, dict_t *xattr, break; } - afr_compound_cleanup (args, xdata, newloc_xdata); + if (xdata) + dict_unref (xdata); + if (newloc_xdata) + dict_unref (newloc_xdata); return 0; err: local->internal_lock.lock_cbk = local->transaction.done; @@ -1411,7 +1418,10 @@ err: afr_restore_lk_owner (frame); afr_unlock (frame, this); - afr_compound_cleanup (args, xdata, newloc_xdata); + if (xdata) + dict_unref (xdata); + if (newloc_xdata) + dict_unref (newloc_xdata); return 0; } @@ -1445,6 +1455,8 @@ afr_post_op_unlock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, UNLOCK (&frame->lock); if (call_count == 0) { + compound_args_cleanup (local->c_args); + local->c_args = NULL; if (local->transaction.resume_stub) { call_resume (local->transaction.resume_stub); local->transaction.resume_stub = NULL; @@ -1526,6 +1538,8 @@ afr_post_op_unlock_do (call_frame_t *frame, xlator_t *this, dict_t *xattr, } } + local->c_args = args; + for (i = 0; i < priv->child_count; i++) { /* pre_op[i] has to be true for all nodes that were * successfully locked. */ @@ -1541,7 +1555,10 @@ afr_post_op_unlock_do (call_frame_t *frame, xlator_t *this, dict_t *xattr, break; } out: - afr_compound_cleanup (args, xdata, newloc_xdata); + if (xdata) + dict_unref (xdata); + if (newloc_xdata) + dict_unref (newloc_xdata); return 0; } diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index dcc162f97c3..eaad64a4f40 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -805,6 +805,7 @@ typedef struct _afr_local { gf_boolean_t need_full_crawl; gf_boolean_t compound; afr_fop_lock_state_t fop_lock_state; + compound_args_t *c_args; } afr_local_t; @@ -1233,10 +1234,6 @@ afr_is_inodelk_transaction(afr_local_t *local); afr_fd_ctx_t * __afr_fd_ctx_get (fd_t *fd, xlator_t *this); -void -afr_compound_cleanup (compound_args_t *args, dict_t *xdata, - dict_t *newloc_xdata); - gf_boolean_t afr_is_inode_refresh_reqd (inode_t *inode, xlator_t *this, int event_gen1, int event_gen2); -- cgit