]> git.ipfire.org Git - thirdparty/rspamd.git/commitdiff
[Fix] fuzzy_storage: avoid per-refresh leak in dynamic ban inserts
authorVsevolod Stakhov <vsevolod@rspamd.com>
Mon, 18 May 2026 09:40:24 +0000 (10:40 +0100)
committerVsevolod Stakhov <vsevolod@rspamd.com>
Mon, 18 May 2026 10:07:55 +0000 (11:07 +0100)
rspamd_fuzzy_block_addr allocated the ban struct from the radix tree's
long-lived mempool before calling radix_insert_compressed.  When the
prefix was already present (the common case: ban_sync re-applies on every
bans_version bump, provisional re-blocks every provisional_ttl), the
btrie rejected the duplicate and the code mutated the existing struct in
place — leaving the freshly allocated one orphaned in the mempool with no
way to reclaim it short of a worker restart.

The pool is created with rspamd_mempool_new_long_lived and freed only at
radix_destroy_compressed, so the orphans accumulate monotonically.  With
thousands of bans churning across a fuzzy fleet and the rspamd-mem-watchdog
trimming workers on a 30-minute cadence, this matches the growth pattern
we have been compensating for.

Look up the prefix first; on a hit, mutate in place without allocating.
Allocate and insert only on a true miss.

src/libserver/fuzzy_storage_ratelimit.c

index 842c45e5bf888f5014d870e56afd1bc46cc6bafa..511f0b08564806706dec8b076dcc9bcdbde86ca8 100644 (file)
@@ -444,18 +444,6 @@ rspamd_fuzzy_block_addr(struct rspamd_fuzzy_storage_ctx *ctx,
        /* Apply network mask to zero out host bits */
        rspamd_inet_address_apply_mask(addr, prefix_len);
 
-       /* Allocate ban struct from the tree's memory pool so it is freed automatically
-        * when the tree is destroyed at shutdown */
-       struct rspamd_fuzzy_dynamic_ban *new_ban =
-               rspamd_mempool_alloc0(radix_get_pool(ctx->dynamic_blocked_nets),
-                                                         sizeof(*new_ban));
-       new_ban->expire_ts = expire_ts;
-       new_ban->response_code = response_code;
-
-       if (reason != NULL) {
-               rspamd_strlcpy(new_ban->reason, reason, sizeof(new_ban->reason));
-       }
-
        /* Build the 16-byte IPv6-mapped key and compute masklen */
        const unsigned char *raw;
        unsigned int klen = 0;
@@ -490,22 +478,39 @@ rspamd_fuzzy_block_addr(struct rspamd_fuzzy_storage_ctx *ctx,
                masklen = 128 - prefix_len;
        }
 
-       uintptr_t old = radix_insert_compressed(ctx->dynamic_blocked_nets,
-                                                                                       key, sizeof(key), masklen,
-                                                                                       (uintptr_t) new_ban);
+       /* Look up the prefix first. On a hit, mutate the existing struct in place
+        * and do not allocate — otherwise every duplicate ban (e.g. ban_sync
+        * re-application, or per-cycle provisional refresh of an already-blocked
+        * /24) leaks one rspamd_fuzzy_dynamic_ban (~80 B) into the radix mempool
+        * with no way to reclaim it short of worker restart. */
+       uintptr_t old = radix_find_compressed(ctx->dynamic_blocked_nets,
+                                                                                 key, sizeof(key));
+       struct rspamd_fuzzy_dynamic_ban *ban;
 
        if (old != RADIX_NO_VALUE) {
-               /* Duplicate prefix: btrie was NOT updated (it returned the existing value).
-                * Update the existing ban struct in-place so the same memory location now
-                * reflects the new TTL and reason. new_ban is pool-allocated so it leaks
-                * a tiny struct, but that is acceptable. */
-               struct rspamd_fuzzy_dynamic_ban *existing = (struct rspamd_fuzzy_dynamic_ban *) old;
-               existing->expire_ts = expire_ts;
+               ban = (struct rspamd_fuzzy_dynamic_ban *) old;
+               ban->expire_ts = expire_ts;
+               ban->response_code = response_code;
 
                if (reason != NULL) {
-                       rspamd_strlcpy(existing->reason, reason, sizeof(existing->reason));
+                       rspamd_strlcpy(ban->reason, reason, sizeof(ban->reason));
                }
        }
+       else {
+               /* New prefix — allocate from the radix mempool (freed at worker
+                * shutdown when the tree is destroyed) and insert. */
+               ban = rspamd_mempool_alloc0(radix_get_pool(ctx->dynamic_blocked_nets),
+                                                                       sizeof(*ban));
+               ban->expire_ts = expire_ts;
+               ban->response_code = response_code;
+
+               if (reason != NULL) {
+                       rspamd_strlcpy(ban->reason, reason, sizeof(ban->reason));
+               }
+
+               radix_insert_compressed(ctx->dynamic_blocked_nets,
+                                                               key, sizeof(key), masklen, (uintptr_t) ban);
+       }
 
        msg_info("dynamic block added for %s/%ud, expire_ts=%.0f, reason=%s",
                         addr_str, prefix_len, expire_ts, reason ? reason : "");