features/bit-rot: fix CID 1124725 - use after free

Coverity fixes:
CID 1124725
CID 1291742

The problem is that gf_tw_cleanup_timers() frees the handed
in priv->timer_wheel but it can not set the pointer to NULL,
so subsequent checks for priv->timer_wheel show it as not NULL
and allow for access after free.

The proper change might be to change gf_tw_cleanup_timers() to
take a reference to the pointer and set it to NULL after free,
but since it is under contrib/, I did not want to change that
function. Instead this patch uses the function's return code
which was not used previously. (Maybe this should even be done
in a wrapper macro or function?)

Change-Id: I31d80d3df2e4dc7503d62c7819429e1a388fdfdd
BUG: 789278
Signed-off-by: Michael Adam <obnox@samba.org>
Reviewed-on: http://review.gluster.org/10056
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Venky Shankar <vshankar@redhat.com>
Tested-by: Venky Shankar <vshankar@redhat.com>

1 files changed, 18 insertions, 4 deletions

diff --git a/xlators/features/bit-rot/src/bitd/bit-rot.c b/xlators/features/bit-rot/src/bitd/bit-rot.c
index 9bd0b8284ec..ba326346cdf 100644
--- a/xlators/features/bit-rot/src/bitd/bit-rot.c
+++ b/xlators/features/bit-rot/src/bitd/bit-rot.c
@@ -1280,18 +1280,23 @@ static inline void
 br_fini_signer (xlator_t *this, br_private_t *priv)
 {
         int i = 0;
+        int ret = 0;
 
         for (; i < BR_WORKERS; i++) {
                 (void) gf_thread_cleanup_xint (priv->obj_queue->workers[i]);
         }
 
         pthread_cond_destroy (&priv->object_cond);
-        gf_tw_cleanup_timers (priv->timer_wheel);
+        ret = gf_tw_cleanup_timers (priv->timer_wheel);
+        if (ret == 0) {
+                priv->timer_wheel = NULL;
+        }
 }
 
 static inline int32_t
 br_init_signer (xlator_t *this, br_private_t *priv)
 {
+        int rc = 0;
         int i = 0;
         int32_t ret = -1;
 
@@ -1338,7 +1343,10 @@ br_init_signer (xlator_t *this, br_private_t *priv)
  cleanup_timer:
         /* that's explicit */
         pthread_cond_destroy (&priv->object_cond);
-        gf_tw_cleanup_timers (priv->timer_wheel);
+        rc = gf_tw_cleanup_timers (priv->timer_wheel);
+        if (rc == 0) {
+                priv->timer_wheel = NULL;
+        }
 
  out:
         return -1;
@@ -1440,6 +1448,7 @@ init (xlator_t *this)
 void
 fini (xlator_t *this)
 {
+        int ret = 0;
 	br_private_t *priv = this->private;
 
         if (!priv)
@@ -1448,8 +1457,13 @@ fini (xlator_t *this)
         if (!priv->iamscrubber)
                 br_fini_signer (this, priv);
         br_free_children (this);
-        if (priv->timer_wheel)
-                gf_tw_cleanup_timers (priv->timer_wheel);
+        if (priv->timer_wheel) {
+                ret = gf_tw_cleanup_timers (priv->timer_wheel);
+
+                if (ret == 0) {
+                        priv->timer_wheel = NULL;
+                }
+        }
 
         this->private = NULL;
 	GF_FREE (priv);