summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMichael Adam <obnox@samba.org>2015-03-30 19:48:27 +0200
committerVenky Shankar <vshankar@redhat.com>2015-04-02 05:27:46 -0700
commit41bc3f7f023de198c695bdb7708afef3910cc761 (patch)
tree429a2b1d06462644a67787259e463c48a0c11a60
parentdb7b8d6e5d4485fc5a6e836447dfa5d3cb85b5fd (diff)
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>
-rw-r--r--xlators/features/bit-rot/src/bitd/bit-rot.c22
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);