commit a165de2: [Fix] Add more checks for ghosts symbols
Vsevolod Stakhov
vsevolod at highsecure.ru
Thu Jul 4 16:28:03 UTC 2019
Author: Vsevolod Stakhov
Date: 2019-07-04 17:22:36 +0100
URL: https://github.com/rspamd/rspamd/commit/a165de2cca6043146c6373e2633bd6d056aff961 (HEAD -> master)
[Fix] Add more checks for ghosts symbols
---
src/libserver/rspamd_symcache.c | 154 ++++++++++++++++++++++++++++------------
1 file changed, 107 insertions(+), 47 deletions(-)
diff --git a/src/libserver/rspamd_symcache.c b/src/libserver/rspamd_symcache.c
index 53e9fd739..326d2bda4 100644
--- a/src/libserver/rspamd_symcache.c
+++ b/src/libserver/rspamd_symcache.c
@@ -139,6 +139,9 @@ struct rspamd_symcache_item {
/* Dependencies */
GPtrArray *deps;
GPtrArray *rdeps;
+
+ /* Container */
+ GPtrArray *container;
};
struct item_stat {
@@ -319,7 +322,7 @@ rspamd_symcache_find_filter (struct rspamd_symcache *cache,
if (item != NULL) {
- if (resolve_parent && item->is_virtual) {
+ if (resolve_parent && item->is_virtual && !(item->type & SYMBOL_TYPE_GHOST)) {
item = g_ptr_array_index (cache->items_by_id,
item->specific.virtual.parent);
}
@@ -346,7 +349,7 @@ rspamd_symcache_get_parent (struct rspamd_symcache *cache,
if (item != NULL) {
- if (item->is_virtual) {
+ if (item->is_virtual && !(item->type & SYMBOL_TYPE_GHOST)) {
item = g_ptr_array_index (cache->items_by_id,
item->specific.virtual.parent);
}
@@ -775,7 +778,7 @@ rspamd_symcache_load_items (struct rspamd_symcache *cache, const gchar *name)
}
}
- if (item->is_virtual) {
+ if (item->is_virtual && !(item->type & SYMBOL_TYPE_GHOST)) {
g_assert (item->specific.virtual.parent < (gint)cache->items_by_id->len);
parent = g_ptr_array_index (cache->items_by_id,
item->specific.virtual.parent);
@@ -919,15 +922,44 @@ rspamd_symcache_add_symbol (struct rspamd_symcache *cache,
}
if (name != NULL && !(type & SYMBOL_TYPE_CALLBACK)) {
- if (g_hash_table_lookup (cache->items_by_symbol, name) != NULL) {
- msg_err_cache ("skip duplicate symbol registration for %s", name);
- return -1;
+ struct rspamd_symcache_item *existing;
+
+ if ((existing = g_hash_table_lookup (cache->items_by_symbol, name)) != NULL) {
+
+ if (existing->type & SYMBOL_TYPE_GHOST) {
+ /*
+ * Complicated part:
+ * - we need to remove the existing ghost symbol
+ * - we need to cleanup containers:
+ * - symbols hash
+ * - specific array
+ * - items_by_it
+ * - decrement used_items
+ */
+ msg_info_cache ("duplicate ghost symbol %s is removed");
+
+ if (existing->container) {
+ g_ptr_array_remove (existing->container, existing);
+ }
+
+ g_ptr_array_remove (cache->items_by_id, existing->container);
+ cache->used_items --;
+ g_hash_table_remove (cache->items_by_symbol, name);
+ /*
+ * Here can be memory leak, but we assume that ghost symbols
+ * are also virtual
+ */
+ }
+ else {
+ msg_err_cache ("skip duplicate symbol registration for %s", name);
+ return -1;
+ }
}
}
if (type & (SYMBOL_TYPE_CLASSIFIER|SYMBOL_TYPE_CALLBACK|
SYMBOL_TYPE_PREFILTER|SYMBOL_TYPE_POSTFILTER|
- SYMBOL_TYPE_IDEMPOTENT)) {
+ SYMBOL_TYPE_IDEMPOTENT|SYMBOL_TYPE_GHOST)) {
type |= SYMBOL_TYPE_NOSTAT;
}
@@ -957,16 +989,20 @@ rspamd_symcache_add_symbol (struct rspamd_symcache *cache,
if (item->type & SYMBOL_TYPE_PREFILTER) {
g_ptr_array_add (cache->prefilters, item);
+ item->container = cache->prefilters;
}
else if (item->type & SYMBOL_TYPE_IDEMPOTENT) {
g_ptr_array_add (cache->idempotent, item);
+ item->container = cache->idempotent;
}
else if (item->type & SYMBOL_TYPE_POSTFILTER) {
g_ptr_array_add (cache->postfilters, item);
+ item->container = cache->postfilters;
}
else {
item->is_filter = TRUE;
g_ptr_array_add (cache->filters, item);
+ item->container = cache->filters;
}
item->id = cache->items_by_id->len;
@@ -979,7 +1015,7 @@ rspamd_symcache_add_symbol (struct rspamd_symcache *cache,
else {
/*
* Three possibilities here when no function is specified:
- * - virtual symbol
+ * - virtual symbol (beware of ghosts!)
* - classifier symbol
* - composite symbol
*/
@@ -991,6 +1027,7 @@ rspamd_symcache_add_symbol (struct rspamd_symcache *cache,
item->id = cache->items_by_id->len;
g_ptr_array_add (cache->items_by_id, item);
+ item->container = cache->composites;
}
else if (item->type & SYMBOL_TYPE_CLASSIFIER) {
/* Treat it as normal symbol to allow enable/disable */
@@ -1007,6 +1044,7 @@ rspamd_symcache_add_symbol (struct rspamd_symcache *cache,
item->specific.virtual.parent = parent;
item->id = cache->virtual->len;
g_ptr_array_add (cache->virtual, item);
+ item->container = cache->virtual;
/* Not added to items_by_id, handled by parent */
}
}
@@ -1270,20 +1308,23 @@ rspamd_symcache_validate_cb (gpointer k, gpointer v, gpointer ud)
}
if (item->is_virtual) {
- g_assert (item->specific.virtual.parent < (gint)cache->items_by_id->len);
- parent = g_ptr_array_index (cache->items_by_id,
- item->specific.virtual.parent);
+ if (!(item->type & SYMBOL_TYPE_GHOST)) {
+ g_assert (item->specific.virtual.parent != 1);
+ g_assert (item->specific.virtual.parent < (gint) cache->items_by_id->len);
+ parent = g_ptr_array_index (cache->items_by_id,
+ item->specific.virtual.parent);
- if (fabs (parent->st->weight) < fabs (item->st->weight)) {
- parent->st->weight = item->st->weight;
- }
+ if (fabs (parent->st->weight) < fabs (item->st->weight)) {
+ parent->st->weight = item->st->weight;
+ }
- p1 = abs (item->priority);
- p2 = abs (parent->priority);
+ p1 = abs (item->priority);
+ p2 = abs (parent->priority);
- if (p1 != p2) {
- parent->priority = MAX (p1, p2);
- item->priority = parent->priority;
+ if (p1 != p2) {
+ parent->priority = MAX (p1, p2);
+ item->priority = parent->priority;
+ }
}
}
@@ -2132,20 +2173,36 @@ rspamd_symcache_counters_cb (gpointer k, gpointer v, gpointer ud)
"symbol", 0, false);
if (item->is_virtual) {
- parent = g_ptr_array_index (cbd->cache->items_by_id,
- item->specific.virtual.parent);
- ucl_object_insert_key (obj,
- ucl_object_fromdouble (ROUND_DOUBLE (item->st->weight)),
- "weight", 0, false);
- ucl_object_insert_key (obj,
- ucl_object_fromdouble (ROUND_DOUBLE (parent->st->avg_frequency)),
- "frequency", 0, false);
- ucl_object_insert_key (obj,
- ucl_object_fromint (parent->st->total_hits),
- "hits", 0, false);
- ucl_object_insert_key (obj,
- ucl_object_fromdouble (ROUND_DOUBLE (parent->st->avg_time)),
- "time", 0, false);
+ if (!(item->type & SYMBOL_TYPE_GHOST)) {
+ parent = g_ptr_array_index (cbd->cache->items_by_id,
+ item->specific.virtual.parent);
+ ucl_object_insert_key (obj,
+ ucl_object_fromdouble (ROUND_DOUBLE (item->st->weight)),
+ "weight", 0, false);
+ ucl_object_insert_key (obj,
+ ucl_object_fromdouble (ROUND_DOUBLE (parent->st->avg_frequency)),
+ "frequency", 0, false);
+ ucl_object_insert_key (obj,
+ ucl_object_fromint (parent->st->total_hits),
+ "hits", 0, false);
+ ucl_object_insert_key (obj,
+ ucl_object_fromdouble (ROUND_DOUBLE (parent->st->avg_time)),
+ "time", 0, false);
+ }
+ else {
+ ucl_object_insert_key (obj,
+ ucl_object_fromdouble (ROUND_DOUBLE (item->st->weight)),
+ "weight", 0, false);
+ ucl_object_insert_key (obj,
+ ucl_object_fromdouble (0.0),
+ "frequency", 0, false);
+ ucl_object_insert_key (obj,
+ ucl_object_fromdouble (0.0),
+ "hits", 0, false);
+ ucl_object_insert_key (obj,
+ ucl_object_fromdouble (0.0),
+ "time", 0, false);
+ }
}
else {
ucl_object_insert_key (obj,
@@ -3300,22 +3357,25 @@ rspamd_symcache_process_settings_elt (struct rspamd_symcache *cache,
if (item) {
if (item->is_virtual) {
- parent = rspamd_symcache_find_filter (cache, sym, true);
-
- if (parent) {
- if (elt->symbols_disabled &&
- ucl_object_lookup (elt->symbols_disabled, parent->symbol)) {
- msg_err_cache ("conflict in %s: cannot enable disabled symbol %s, "
- "wanted to enable symbol %s",
- elt->name, parent->symbol, sym);
- continue;
+ if (!(item->type & SYMBOL_TYPE_GHOST)) {
+ parent = rspamd_symcache_find_filter (cache, sym, true);
+
+ if (parent) {
+ if (elt->symbols_disabled &&
+ ucl_object_lookup (elt->symbols_disabled, parent->symbol)) {
+ msg_err_cache ("conflict in %s: cannot enable disabled symbol %s, "
+ "wanted to enable symbol %s",
+ elt->name, parent->symbol, sym);
+ continue;
+ }
+
+ rspamd_symcache_add_id_to_list (cache->static_pool,
+ &parent->exec_only_ids, id);
+ msg_debug_cache ("allow just execution of symbol %s for settings %ud (%s)",
+ parent->symbol, id, elt->name);
}
-
- rspamd_symcache_add_id_to_list (cache->static_pool,
- &parent->exec_only_ids, id);
- msg_debug_cache ("allow just execution of symbol %s for settings %ud (%s)",
- parent->symbol, id, elt->name);
}
+ /* Ignore ghosts */
}
rspamd_symcache_add_id_to_list (cache->static_pool,
More information about the Commits
mailing list