Hello,

CypherFox <openbsd@cypher-fox.com> wrote:
> Hello,
> 
> I found a memory leak in relayd's config_purge() when handling 
> CONFIG_PROTOS.
> The current implementation uses two separate loops: the first one 
> empties the
> sc_protos list, which causes the second loop (responsible for the actual
> free() calls) to be skipped entirely.
> 
> I've verified this with lldb. The second loop is never entered because
> TAILQ_FIRST(env->sc_protos) returns NULL after the first loop finishes,
> leaving the protocol structures and their members orphaned in the heap:
> 
> 
> (lldb) p *env->sc_protos
> (protolist) {
>    tqh_first = NULL
>    tqh_last = 0x000007fcaf00ee30
> }
> (lldb) p *env->sc_protos->tqh_last
> (protocol *) NULL
> (lldb) p ((struct protocol *)0x000007fc3db03000)->tlscapass
> (char *) 0x000007fcaf003360 "s3cr3t"
> 
> 
> This patch unifies the logic into a single loop to ensure all resources 
> (rules, tlscerts, styles, and the protocol structure) are properly freed.

Makes sense, good catch!

I've just rearranged the diff to be a tiny smaller (and completely
net-negative!), and committed it.  thanks!

diff /usr/src
path + /usr/src
commit - 56d54b2a7cc014527ea8ba0e2bacd270917079ed
blob - 60bdb00b63babb44353e19f25009d86ba1a91a20
file + usr.sbin/relayd/config.c
--- usr.sbin/relayd/config.c
+++ usr.sbin/relayd/config.c
@@ -183,17 +183,12 @@ config_purge(struct relayd *env, u_int reset)
 	if (what & CONFIG_PROTOS && env->sc_protos != NULL) {
 		while ((proto = TAILQ_FIRST(env->sc_protos)) != NULL) {
 			TAILQ_REMOVE(env->sc_protos, proto, entry);
 			while ((rule = TAILQ_FIRST(&proto->rules)) != NULL)
 				rule_delete(&proto->rules, rule);
 			proto->rulecount = 0;
-		}
-	}
-	if (what & CONFIG_PROTOS && env->sc_protos != NULL) {
-		while ((proto = TAILQ_FIRST(env->sc_protos)) != NULL) {
-			TAILQ_REMOVE(env->sc_protos, proto, entry);
 			free(proto->style);
 			free(proto->tlscapass);
 			while ((keyname =
 			    TAILQ_FIRST(&proto->tlscerts)) != NULL) {
 				TAILQ_REMOVE(&proto->tlscerts, keyname, entry);
 				free(keyname->name);