From 37897f0b72617e442e4799b35ebda94294218e05 Mon Sep 17 00:00:00 2001

From: Pranith Kumar K <pkarampu@redhat.com>

Date: Wed, 28 Feb 2018 17:58:31 +0530

Subject: [PATCH 183/201] cluster/afr: Fix dict-leak in pre-op

At the time of pre-op, pre_op_xdata is populted with the xattrs we get from the

disk and at the time of post-op it gets over-written without unreffing the

previous value stored leading to a leak.

This is a regression we missed in

https://review.gluster.org/#/q/ba149bac92d169ae2256dbc75202dc9e5d06538e

 >BUG: 1550078

 >Change-Id: I0456f9ad6f77ce6248b747964a037193af3a3da7

 >Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>

 >Upstream: https://review.gluster.org/19647

BUG: 1552360

Change-Id: I0456f9ad6f77ce6248b747964a037193af3a3da7

Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/131936

Tested-by: RHGS Build Bot <nigelb@redhat.com>

Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>

---

 xlators/cluster/afr/src/afr-common.c      | 16 ++++++++--------

 xlators/cluster/afr/src/afr-transaction.c | 20 ++++++++++----------

 xlators/cluster/afr/src/afr.h             |  4 ++--

 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c

index 6e6f5fa..855e568 100644

--- a/xlators/cluster/afr/src/afr-common.c

+++ b/xlators/cluster/afr/src/afr-common.c

@@ -1039,7 +1039,7 @@ afr_readables_fill (call_frame_t *frame, xlator_t *this, inode_t *inode,

                         xdata = replies[i].xdata;

                         ia_type = replies[i].poststat.ia_type;

                 } else {/* pre-op xattrop */

-                        xdata = local->transaction.pre_op_xdata[i];

+                        xdata = local->transaction.changelog_xdata[i];

                         ia_type = inode->ia_type;

                 }

@@ -1757,13 +1757,13 @@ afr_local_transaction_cleanup (afr_local_t *local, xlator_t *this)

         GF_FREE (local->transaction.pre_op);

         GF_FREE (local->transaction.pre_op_sources);

-        if (local->transaction.pre_op_xdata) {

+        if (local->transaction.changelog_xdata) {

                 for (i = 0; i < priv->child_count; i++) {

-                        if (!local->transaction.pre_op_xdata[i])

+                        if (!local->transaction.changelog_xdata[i])

                                 continue;

-                        dict_unref (local->transaction.pre_op_xdata[i]);

+                        dict_unref (local->transaction.changelog_xdata[i]);

                 }

-                GF_FREE (local->transaction.pre_op_xdata);

+                GF_FREE (local->transaction.changelog_xdata);

         }

         GF_FREE (local->transaction.eager_lock);

@@ -5531,10 +5531,10 @@ afr_transaction_local_init (afr_local_t *local, xlator_t *this)

         if (!local->transaction.pre_op)

                 goto out;

-        local->transaction.pre_op_xdata =

-                GF_CALLOC (sizeof (*local->transaction.pre_op_xdata),

+        local->transaction.changelog_xdata =

+                GF_CALLOC (sizeof (*local->transaction.changelog_xdata),

                            priv->child_count, gf_afr_mt_dict_t);

-        if (!local->transaction.pre_op_xdata)

+        if (!local->transaction.changelog_xdata)

                 goto out;

         if (priv->arbiter_count == 1) {

diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c

index 19740e1..97f9dd4 100644

--- a/xlators/cluster/afr/src/afr-transaction.c

+++ b/xlators/cluster/afr/src/afr-transaction.c

@@ -276,9 +276,9 @@ afr_compute_pre_op_sources (call_frame_t *frame, xlator_t *this)

         matrix = ALLOC_MATRIX (priv->child_count, int);

         for (i = 0; i < priv->child_count; i++) {

-                if (!local->transaction.pre_op_xdata[i])

+                if (!local->transaction.changelog_xdata[i])

                         continue;

-                xdata = local->transaction.pre_op_xdata[i];

+                xdata = local->transaction.changelog_xdata[i];

                 afr_selfheal_fill_matrix (this, matrix, i, idx, xdata);

         }

@@ -295,13 +295,6 @@ afr_compute_pre_op_sources (call_frame_t *frame, xlator_t *this)

                 for (j = 0; j < priv->child_count; j++)

                         if (matrix[i][j] != 0)

                                 local->transaction.pre_op_sources[j] = 0;

-

-        /*We don't need the xattrs any more. */

-        for (i = 0; i < priv->child_count; i++)

-                if (local->transaction.pre_op_xdata[i]) {

-                        dict_unref (local->transaction.pre_op_xdata[i]);

-                        local->transaction.pre_op_xdata[i] = NULL;

-                }

 }

 gf_boolean_t

@@ -1173,7 +1166,7 @@ afr_changelog_cbk (call_frame_t *frame, void *cookie, xlator_t *this,

         }

         if (xattr)

-                local->transaction.pre_op_xdata[child_index] = dict_ref (xattr);

+                local->transaction.changelog_xdata[child_index] = dict_ref (xattr);

         call_count = afr_frame_return (frame);

@@ -1605,6 +1598,13 @@ afr_changelog_do (call_frame_t *frame, xlator_t *this, dict_t *xattr,

 	local = frame->local;

 	priv = this->private;

+        for (i = 0; i < priv->child_count; i++) {

+                if (local->transaction.changelog_xdata[i]) {

+                        dict_unref (local->transaction.changelog_xdata[i]);

+                        local->transaction.changelog_xdata[i] = NULL;

+                }

+        }

+

         ret = afr_changelog_prepare (this, frame, &call_count, changelog_resume,

                                      op, &xdata, &newloc_xdata);

diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h

index 96fefb1..c822221 100644

--- a/xlators/cluster/afr/src/afr.h

+++ b/xlators/cluster/afr/src/afr.h

@@ -748,8 +748,8 @@ typedef struct _afr_local {

                 unsigned char   *pre_op;

-                /* For arbiter configuration only. */

-                dict_t **pre_op_xdata;

+                /* Changelog xattr dict for [f]xattrop*/

+                dict_t **changelog_xdata;

                 unsigned char *pre_op_sources;

 		/* @failed_subvols: subvolumes on which a pre-op or a

-- 

1.8.3.1