all: handle USE_AFTER_FREE warnings

* we shouldn't be using 'local' after DHT_STACK_UNWIND() as it frees
the content of local. Add a 'goto out' or similar logic to handle
the situation.

* fix possible overlook of unref(dict), instead of unref(xdata).

* make coverity happy by re-ordering unref in meta-defaults.

* gfid-access: re-order dictionary allocation so we don't have to
  do a extra unref.

* other obvious errors reported.

updates: bz#789278
Change-Id: If05961ee946b0c4868df19861d7e4a927a2a2489
Signed-off-by: Amar Tumballi <amarts@redhat.com>

6 files changed, 60 insertions, 16 deletions

diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index 985e7580050..b6e4fbecf8a 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -3568,17 +3568,21 @@ gf_is_local_addr(char *hostname)
     }
 
     for (res = result; res != NULL; res = res->ai_next) {
-        gf_msg_debug(this->name, 0, "%s ", get_ip_from_addrinfo(res, &ip));
+        get_ip_from_addrinfo(res, &ip);
+        gf_msg_debug(this->name, 0, "%s ", ip);
 
         if (ip) {
-            found = gf_is_loopback_localhost(res->ai_addr, hostname) ||
-                    gf_interface_search(ip);
+            found = (gf_is_loopback_localhost(res->ai_addr, hostname) ||
+                     gf_interface_search(ip));
         }
         if (found) {
             GF_FREE(ip);
             goto out;
         }
         GF_FREE(ip);
+        /* the above free will not set ip to NULL, and hence, there is
+           double free possible as the loop continues. set ip to NULL. */
+        ip = NULL;
     }
 
 out:
diff --git a/libglusterfs/src/rbthash.c b/libglusterfs/src/rbthash.c
index f5cd6a3d88c..1bdd45f9fae 100644
--- a/libglusterfs/src/rbthash.c
+++ b/libglusterfs/src/rbthash.c
@@ -286,6 +286,7 @@ rbthash_insert(rbthash_table_t *tbl, void *data, void *key, int keylen)
         gf_msg(GF_RBTHASH, GF_LOG_ERROR, 0, LG_MSG_RBTHASH_INSERT_FAILED,
                "Failed to insert entry");
         rbthash_deinit_entry(tbl, entry);
+        goto err;
     }
 
     LOCK(&tbl->tablelock);
diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c
index 86da0c3f935..d886202ced7 100644
--- a/xlators/cluster/dht/src/dht-common.c
+++ b/xlators/cluster/dht/src/dht-common.c
@@ -3705,6 +3705,8 @@ unlock:
             (local->fop == GF_FOP_FSETXATTR)) {
             DHT_STACK_UNWIND(setxattr, frame, local->op_ret, local->op_errno,
                              NULL);
+            /* 'local' itself may not be valid after this */
+            goto out;
         }
         if ((local->fop == GF_FOP_REMOVEXATTR) ||
             (local->fop == GF_FOP_FREMOVEXATTR)) {
@@ -3713,6 +3715,7 @@ unlock:
         }
     }
 
+out:
     return 0;
 }
 
@@ -3759,20 +3762,27 @@ dht_common_mds_xattrop_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
 
     if (local->fop == GF_FOP_SETXATTR) {
         DHT_STACK_UNWIND(setxattr, frame, 0, op_errno, local->xdata);
+        /* 'local' itself may not be valid after this */
+        goto out;
     }
 
     if (local->fop == GF_FOP_FSETXATTR) {
         DHT_STACK_UNWIND(fsetxattr, frame, 0, op_errno, local->xdata);
+        /* 'local' itself may not be valid after this */
+        goto out;
     }
 
     if (local->fop == GF_FOP_REMOVEXATTR) {
         DHT_STACK_UNWIND(removexattr, frame, 0, op_errno, NULL);
+        /* 'local' itself may not be valid after this */
+        goto out;
     }
 
     if (local->fop == GF_FOP_FREMOVEXATTR) {
         DHT_STACK_UNWIND(fremovexattr, frame, 0, op_errno, NULL);
     }
 
+out:
     return 0;
 }
 
@@ -3836,41 +3846,56 @@ dht_setxattr_non_mds_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         } else {
             if (local->fop == GF_FOP_SETXATTR) {
                 DHT_STACK_UNWIND(setxattr, frame, 0, 0, local->xdata);
+                /* 'local' itself may not be valid after this */
+                goto just_return;
             }
 
             if (local->fop == GF_FOP_FSETXATTR) {
                 DHT_STACK_UNWIND(fsetxattr, frame, 0, 0, local->xdata);
+                /* 'local' itself may not be valid after this */
+                goto just_return;
             }
 
             if (local->fop == GF_FOP_REMOVEXATTR) {
                 DHT_STACK_UNWIND(removexattr, frame, 0, 0, NULL);
+                /* 'local' itself may not be valid after this */
+                goto just_return;
             }
 
             if (local->fop == GF_FOP_FREMOVEXATTR) {
                 DHT_STACK_UNWIND(fremovexattr, frame, 0, 0, NULL);
+                /* 'local' itself may not be valid after this */
+                goto just_return;
             }
         }
     }
 out:
-    if (xattrop)
-        dict_unref(xattrop);
     if (ret) {
         if (local->fop == GF_FOP_SETXATTR) {
             DHT_STACK_UNWIND(setxattr, frame, 0, 0, local->xdata);
+            /* 'local' itself may not be valid after this */
+            goto just_return;
         }
 
         if (local->fop == GF_FOP_FSETXATTR) {
             DHT_STACK_UNWIND(fsetxattr, frame, 0, 0, local->xdata);
+            /* 'local' itself may not be valid after this */
+            goto just_return;
         }
 
         if (local->fop == GF_FOP_REMOVEXATTR) {
             DHT_STACK_UNWIND(removexattr, frame, 0, 0, NULL);
+            /* 'local' itself may not be valid after this */
+            goto just_return;
         }
 
         if (local->fop == GF_FOP_FREMOVEXATTR) {
             DHT_STACK_UNWIND(fremovexattr, frame, 0, 0, NULL);
         }
     }
+just_return:
+    if (xattrop)
+        dict_unref(xattrop);
     return 0;
 }
 
@@ -3934,16 +3959,22 @@ out:
     if (local->fop == GF_FOP_SETXATTR) {
         DHT_STACK_UNWIND(setxattr, frame, local->op_ret, local->op_errno,
                          xdata);
+        /* 'local' itself may not be valid after this */
+        goto just_return;
     }
 
     if (local->fop == GF_FOP_FSETXATTR) {
         DHT_STACK_UNWIND(fsetxattr, frame, local->op_ret, local->op_errno,
                          xdata);
+        /* 'local' itself may not be valid after this */
+        goto just_return;
     }
 
     if (local->fop == GF_FOP_REMOVEXATTR) {
         DHT_STACK_UNWIND(removexattr, frame, local->op_ret, local->op_errno,
                          NULL);
+        /* 'local' itself may not be valid after this */
+        goto just_return;
     }
 
     if (local->fop == GF_FOP_FREMOVEXATTR) {
@@ -3951,6 +3982,7 @@ out:
                          NULL);
     }
 
+just_return:
     return 0;
 }
 
@@ -4001,16 +4033,22 @@ out:
     if (local->fop == GF_FOP_SETXATTR) {
         DHT_STACK_UNWIND(setxattr, frame, local->op_ret, local->op_errno,
                          xdata);
+        /* 'local' itself may not be valid after this */
+        goto just_return;
     }
 
     if (local->fop == GF_FOP_FSETXATTR) {
         DHT_STACK_UNWIND(fsetxattr, frame, local->op_ret, local->op_errno,
                          xdata);
+        /* 'local' itself may not be valid after this */
+        goto just_return;
     }
 
     if (local->fop == GF_FOP_REMOVEXATTR) {
         DHT_STACK_UNWIND(removexattr, frame, local->op_ret, local->op_errno,
                          NULL);
+        /* 'local' itself may not be valid after this */
+        goto just_return;
     }
 
     if (local->fop == GF_FOP_FREMOVEXATTR) {
@@ -4018,6 +4056,7 @@ out:
                          NULL);
     }
 
+just_return:
     return 0;
 }
 
diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c
index 187244435d6..46acc77c4b0 100644
--- a/xlators/cluster/dht/src/dht-rebalance.c
+++ b/xlators/cluster/dht/src/dht-rebalance.c
@@ -875,7 +875,7 @@ out:
         dict_unref(dict);
 
     if (xdata)
-        dict_unref(dict);
+        dict_unref(xdata);
 
     return ret;
 }
diff --git a/xlators/features/gfid-access/src/gfid-access.c b/xlators/features/gfid-access/src/gfid-access.c
index 4a422ee658c..ad7776741d9 100644
--- a/xlators/features/gfid-access/src/gfid-access.c
+++ b/xlators/features/gfid-access/src/gfid-access.c
@@ -448,14 +448,6 @@ ga_new_entry(call_frame_t *frame, xlator_t *this, loc_t *loc, data_t *data,
         0,
     };
 
-    args = ga_newfile_parse_args(this, data);
-    if (!args)
-        goto out;
-
-    ret = gf_uuid_parse(args->gfid, gfid);
-    if (ret)
-        goto out;
-
     if (!xdata) {
         xdata = dict_new();
     } else {
@@ -467,6 +459,14 @@ ga_new_entry(call_frame_t *frame, xlator_t *this, loc_t *loc, data_t *data,
         goto out;
     }
 
+    args = ga_newfile_parse_args(this, data);
+    if (!args)
+        goto out;
+
+    ret = gf_uuid_parse(args->gfid, gfid);
+    if (ret)
+        goto out;
+
     ret = ga_fill_tmp_loc(loc, this, gfid, args->bname, xdata, &tmp_loc);
     if (ret)
         goto out;
diff --git a/xlators/meta/src/meta-defaults.c b/xlators/meta/src/meta-defaults.c
index 12dc5f484b6..ea8f3230b1d 100644
--- a/xlators/meta/src/meta-defaults.c
+++ b/xlators/meta/src/meta-defaults.c
@@ -145,11 +145,11 @@ meta_default_readv(call_frame_t *frame, xlator_t *this, fd_t *fd, size_t size,
         return default_readv_failure_cbk(frame, ENOMEM);
     }
 
+    iov.iov_base = iobuf_ptr(iobuf);
+
     /* iobref would have taken a ref */
     iobuf_unref(iobuf);
 
-    iov.iov_base = iobuf_ptr(iobuf);
-
     copy_offset = min(meta_fd->size, offset);
     copy_size = min(size, (meta_fd->size - copy_offset));