From 41bc3f7f023de198c695bdb7708afef3910cc761 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Mon, 30 Mar 2015 19:48:27 +0200 Subject: 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 Reviewed-on: http://review.gluster.org/10056 Tested-by: Gluster Build System Reviewed-by: Venky Shankar Tested-by: Venky Shankar --- xlators/features/bit-rot/src/bitd/bit-rot.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'xlators/features/bit-rot/src/bitd/bit-rot.c') 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); -- cgit