From: Florian Forster Date: Fri, 6 Mar 2009 15:53:28 +0000 (+0100) Subject: rrdtool plugin: Fix a possible race condition at startup. X-Git-Tag: collectd-4.6.2~5^2~2 X-Git-Url: https://git.octo.it/?p=collectd.git;a=commitdiff_plain;h=4e57613ae19a71a0f180f0648223a139f48c932c rrdtool plugin: Fix a possible race condition at startup. On a very busy system, if the network plugin was initialized before the rrdtool plugin, `rrd_cache_insert' may be called before the RRDtool plugin is initialized. It would then pass `cache == NULL' to `c_avl_get', resulting in a segmentation fault. --- diff --git a/src/rrdtool.c b/src/rrdtool.c index e1c7902b..27e64c11 100644 --- a/src/rrdtool.c +++ b/src/rrdtool.c @@ -621,6 +621,7 @@ static void *rrd_queue_thread (void *data) rrd_cache_t *cache_entry; char **values; int values_num; + int status; int i; pthread_mutex_lock (&queue_lock); @@ -628,7 +629,6 @@ static void *rrd_queue_thread (void *data) while (true) { struct timespec ts_wait; - int status; while ((flushq_head == NULL) && (queue_head == NULL) && (do_shutdown == 0)) @@ -704,17 +704,28 @@ static void *rrd_queue_thread (void *data) * we make a copy of it's values */ pthread_mutex_lock (&cache_lock); - c_avl_get (cache, queue_entry->filename, (void *) &cache_entry); + status = c_avl_get (cache, queue_entry->filename, + (void *) &cache_entry); - values = cache_entry->values; - values_num = cache_entry->values_num; + if (status == 0) + { + values = cache_entry->values; + values_num = cache_entry->values_num; - cache_entry->values = NULL; - cache_entry->values_num = 0; - cache_entry->flags = FLAG_NONE; + cache_entry->values = NULL; + cache_entry->values_num = 0; + cache_entry->flags = FLAG_NONE; + } pthread_mutex_unlock (&cache_lock); + if (status != 0) + { + sfree (queue_entry->filename); + sfree (queue_entry); + continue; + } + /* Update `tv_next_update' */ if (write_rate > 0.0) { @@ -968,6 +979,15 @@ static int rrd_cache_insert (const char *filename, pthread_mutex_lock (&cache_lock); + /* This shouldn't happen, but it did happen at least once, so we'll be + * careful. */ + if (cache == NULL) + { + pthread_mutex_unlock (&cache_lock); + WARNING ("rrdtool plugin: cache == NULL."); + return (-1); + } + c_avl_get (cache, filename, (void *) &rc); if (rc == NULL) diff --git a/src/utils_avltree.c b/src/utils_avltree.c index 9f0b7968..3e258e95 100644 --- a/src/utils_avltree.c +++ b/src/utils_avltree.c @@ -581,6 +581,8 @@ int c_avl_get (c_avl_tree_t *t, const void *key, void **value) { c_avl_node_t *n; + assert (t != NULL); + n = search (t, key); if (n == NULL) return (-1);