From: Vsevolod Stakhov <vsevolod@rspamd.com>
Date: Tue, 28 Oct 2025 14:37:36 +0000 (+0000)
Subject: [Fix] Fix memory leak in upstream address parsing
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=20bdd0ce2137458d23a84fcfbe4de16a4e6ed152;p=thirdparty%2Frspamd.git

[Fix] Fix memory leak in upstream address parsing

When parsing upstream addresses in rspamd_upstreams_add_upstream(),
the GPtrArray 'addrs' was not properly freed in several cases:

1. For service= variant: destructor was not added to mempool
2. After copying addresses to upstream: array was not freed when
   ups->ctx is NULL
3. On parse failure: addrs was not freed

This commit adds proper cleanup:
- Add mempool destructor for service= variant when ctx exists
- Free addrs after copying addresses if no ctx (not managed by mempool)
- Free addrs on parse failure if not managed by mempool
- Set addrs = NULL after freeing to avoid dangling pointer

The fix is careful about mempool-managed vs manual cleanup to avoid
double-free issues, especially important since upstreams can be called
from destructor functions.
---

diff --git a/src/libutil/upstream.c b/src/libutil/upstream.c
index 85964c22a6..acbc05736e 100644
--- a/src/libutil/upstream.c
+++ b/src/libutil/upstream.c
@@ -1304,6 +1304,12 @@ rspamd_upstreams_add_upstream(struct upstream_list *ups, const char *str,
 								(int) (semicolon_pos - (plus_pos + 1)), plus_pos + 1);
 				upstream->flags |= RSPAMD_UPSTREAM_FLAG_SRV_RESOLVE;
 				ret = RSPAMD_PARSE_ADDR_RESOLVED;
+
+				if (ups->ctx) {
+					rspamd_mempool_add_destructor(ups->ctx->pool,
+												  (rspamd_mempool_destruct_t) rspamd_ptr_array_free_hard,
+												  addrs);
+				}
 			}
 		}
 		else {
@@ -1390,6 +1396,7 @@ rspamd_upstreams_add_upstream(struct upstream_list *ups, const char *str,
 			}
 			else {
 				g_ptr_array_free(addrs, TRUE);
+				addrs = NULL;
 			}
 		}
 
@@ -1397,6 +1404,10 @@ rspamd_upstreams_add_upstream(struct upstream_list *ups, const char *str,
 	}
 
 	if (ret == RSPAMD_PARSE_ADDR_FAIL) {
+		/* Clean up addrs if created but not managed by mempool */
+		if (addrs && !ups->ctx) {
+			g_ptr_array_free(addrs, TRUE);
+		}
 		g_free(upstream);
 		return FALSE;
 	}
@@ -1411,6 +1422,11 @@ rspamd_upstreams_add_upstream(struct upstream_list *ups, const char *str,
 			addr = g_ptr_array_index(addrs, i);
 			rspamd_upstream_add_addr(upstream, rspamd_inet_address_copy(addr, NULL));
 		}
+
+		/* Free addrs array if no pool (not managed by mempool destructor) */
+		if (!ups->ctx) {
+			g_ptr_array_free(addrs, TRUE);
+		}
 	}
 
 	if (upstream->weight == 0 && ups->rot_alg == RSPAMD_UPSTREAM_MASTER_SLAVE) {