features/posix : Avoid double free of a variable in posix_setxattr()

The buffer acl_xattr is introduced in posix_setxattr() as part of
http://review.gluster.org/#/c/11519/. This variable can be freed
twice in the code path , one in dict_unref() and another by explicit
GF_FREE() call in the code. This patch avoids the same.

Backport of http://review.gluster.org/#/c/11627/

>Change-Id: I31c6384e37ab8d8baaed7a53de668c2eb5d82338
>BUG: 1242030
>Signed-off-by: Jiffin Tony Thottan <jthottan@redhat.com>

Change-Id: I172be30c0570fe096138c2e529c45fb26497f649
BUG: 1242031
Signed-off-by: Jiffin Tony Thottan <jthottan@redhat.com>
Reviewed-on: http://review.gluster.org/11628
Tested-by: NetBSD Build System <jenkins@build.gluster.org>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>

1 files changed, 24 insertions, 24 deletions

diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c
index 46c69e6898f..81845f81795 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -3343,10 +3343,8 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
                  * reduced into required size using GF_REALLO().
                  */
                 acl_xattr = GF_CALLOC (1, ACL_BUFFER_MAX, gf_posix_mt_char);
-                if (!acl_xattr) {
-                        ret = -1;
+                if (!acl_xattr)
                         goto out;
-                }
 
                 acl_size = sys_lgetxattr (real_path, POSIX_ACL_ACCESS_XATTR,
                                           acl_xattr, ACL_BUFFER_MAX);
@@ -3355,7 +3353,6 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
                         gf_msg (this->name, GF_LOG_WARNING, errno,
                                 P_MSG_XATTR_FAILED, "Posix acl is not set "
                                 "properly at the backend");
-                        ret = -1;
                         goto out;
                 }
 
@@ -3364,32 +3361,33 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
                         gf_msg (this->name, GF_LOG_WARNING, ENOMEM,
                                 P_MSG_BUFFER_OVERFLOW, "size of acl is more"
                                 "than the buffer");
-                        ret = -1;
                         goto out;
                 }
 
                 acl_xattr = GF_REALLOC (acl_xattr, acl_size);
+                if (!acl_xattr)
+                        goto out;
+
                 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");
-                        ret = -1;
-                        goto out;
-                }
-        }
 
-        GF_FREE (acl_xattr);
-        acl_xattr = NULL;
+                /*
+                 * 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;
+        }
 
         if (dict_get (dict, GF_POSIX_ACL_DEFAULT)) {
 
                 acl_xattr = GF_CALLOC (1, ACL_BUFFER_MAX, gf_posix_mt_char);
-                if (!acl_xattr) {
-                        ret = -1;
+                if (!acl_xattr)
                         goto out;
-                }
+
                 acl_size = sys_lgetxattr (real_path, POSIX_ACL_DEFAULT_XATTR,
                                           acl_xattr, ACL_BUFFER_MAX);
 
@@ -3397,7 +3395,6 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
                         gf_msg (this->name, GF_LOG_WARNING, errno,
                                 P_MSG_XATTR_FAILED, "Posix acl is not set "
                                 "properly at the backend");
-                        ret = -1;
                         goto out;
                 }
 
@@ -3405,20 +3402,25 @@ posix_setxattr (call_frame_t *frame, xlator_t *this,
                         gf_msg (this->name, GF_LOG_WARNING, ENOMEM,
                                 P_MSG_BUFFER_OVERFLOW, "size of acl is more"
                                 "than the buffer");
-                        ret = -1;
                         goto out;
                 }
 
                 acl_xattr = GF_REALLOC (acl_xattr, acl_size);
+                if (!acl_xattr)
+                        goto out;
+
                 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");
-                        ret = -1;
-                        goto out;
-                }
+
+                /*
+                 * 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;
         }
 out:
         SET_TO_OLD_FS_ID ();
@@ -3427,9 +3429,7 @@ out:
 
         if (xattr)
                 dict_unref(xattr);
-
-        if (acl_xattr)
-                GF_FREE (acl_xattr);
+        GF_FREE (acl_xattr);
 
         return 0;
 }