From: Aurelien DARRAGON Date: Mon, 17 Jun 2024 16:53:48 +0000 (+0200) Subject: BUG/MINOR: proxy: fix email-alert leak on deinit() (2nd try) X-Git-Tag: v3.1-dev2~37 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9d312212dfa3eaf678c5fabcc6f1045192b8ef19;p=thirdparty%2Fhaproxy.git BUG/MINOR: proxy: fix email-alert leak on deinit() (2nd try) As shown in GH #2608 and ("BUG/MEDIUM: proxy: fix email-alert invalid free"), simply calling free_email_alert() from free_proxy() is not the right thing to do. In this patch, we reuse proxy->email_alert.set memory space to introduce proxy->email_alert.flags in order to support 2 flags: PR_EMAIL_ALERT_SET (to mimic proxy->email_alert.set) and PR_EMAIL_ALERT_RESOLVED (set once init_email_alert() was called on the proxy to resolve email_alert.mailer pointer). Thanks to PR_EMAIL_ALERT_RESOLVED flag, free_email_alert() may now properly handle the freeing of proxy email_alert settings: if the RESOLVED flag is set, then it means the .email_alert.mailers.name parsing hint was replaced by the actual mailers pointer, thus no free should be attempted. No backport needed: as described in ("BUG/MEDIUM: proxy: fix email-alert invalid free"), this historical leak is not sensitive as it cannot be triggered during runtime.. thus given that the fix is not backport- friendly, it's not worth the trouble. --- diff --git a/include/haproxy/mailers-t.h b/include/haproxy/mailers-t.h index 0fa3197a97..5df95a977d 100644 --- a/include/haproxy/mailers-t.h +++ b/include/haproxy/mailers-t.h @@ -35,6 +35,13 @@ #include #include +/* flags for proxy.email_alert.flags */ +enum proxy_email_alert_flags { + PR_EMAIL_ALERT_NONE = 0, + PR_EMAIL_ALERT_SET, /* set if email alert settings are present */ + PR_EMAIL_ALERT_RESOLVED, /* set if email alert settings were resolved */ +}; + struct mailer { char *id; struct mailers *mailers; diff --git a/include/haproxy/proxy-t.h b/include/haproxy/proxy-t.h index f6ed211c14..262dc11d5e 100644 --- a/include/haproxy/proxy-t.h +++ b/include/haproxy/proxy-t.h @@ -438,7 +438,7 @@ struct proxy { char *myhostname; /* Identity to use in HELO command sent to mailer */ int level; /* Maximum syslog level of messages to send * email alerts for */ - int set; /* True if email_alert settings are present */ + int flags; /* check mailers.h for available flags */ struct email_alertq *queues; /* per-mailer alerts queues */ } email_alert; diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c index 9ee81742c0..1f6ddc2adc 100644 --- a/src/cfgparse-listen.c +++ b/src/cfgparse-listen.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -974,7 +975,7 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm) goto out; } /* Indicate that the email_alert is at least partially configured */ - curproxy->email_alert.set = 1; + curproxy->email_alert.flags |= PR_EMAIL_ALERT_SET; }/* end else if (!strcmp(args[0], "email-alert")) */ else if (strcmp(args[0], "persist") == 0) { /* persist */ if (*(args[1]) == 0) { diff --git a/src/cfgparse.c b/src/cfgparse.c index f7f6481883..2434de2cb6 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -3067,7 +3067,7 @@ init_proxies_list_stage1: } } - if (curproxy->email_alert.set) { + if (curproxy->email_alert.flags & PR_EMAIL_ALERT_SET) { if (!(curproxy->email_alert.mailers.name && curproxy->email_alert.from && curproxy->email_alert.to)) { ha_warning("'email-alert' will be ignored for %s '%s' (the presence any of " "'email-alert from', 'email-alert level' 'email-alert mailers', " diff --git a/src/mailers.c b/src/mailers.c index 85c18eebd5..905bb6745a 100644 --- a/src/mailers.c +++ b/src/mailers.c @@ -157,6 +157,7 @@ int init_email_alert(struct mailers *mls, struct proxy *p, char **err) mls->users++; free(p->email_alert.mailers.name); p->email_alert.mailers.m = mls; + p->email_alert.flags |= PR_EMAIL_ALERT_RESOLVED; p->email_alert.queues = queues; return 0; @@ -174,7 +175,8 @@ int init_email_alert(struct mailers *mls, struct proxy *p, char **err) void free_email_alert(struct proxy *p) { - ha_free(&p->email_alert.mailers.name); + if (!(p->email_alert.flags & PR_EMAIL_ALERT_RESOLVED)) + ha_free(&p->email_alert.mailers.name); ha_free(&p->email_alert.from); ha_free(&p->email_alert.to); ha_free(&p->email_alert.myhostname); diff --git a/src/proxy.c b/src/proxy.c index a29f8a5261..10cb780257 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -254,6 +254,8 @@ static inline void proxy_free_common(struct proxy *px) LIST_DEL_INIT(&lf->list); chunk_destroy(&px->log_tag); + + free_email_alert(px); } void free_proxy(struct proxy *p) @@ -1482,7 +1484,6 @@ void proxy_free_defaults(struct proxy *defproxy) proxy_release_conf_errors(defproxy); deinit_proxy_tcpcheck(defproxy); - free_email_alert(defproxy); /* FIXME: we cannot free uri_auth because it might already be used by * another proxy (legacy code for stats URI ...). Refcount anyone ? @@ -1791,6 +1792,7 @@ static int proxy_defproxy_cpy(struct proxy *curproxy, const struct proxy *defpro if (defproxy->check_command) curproxy->check_command = strdup(defproxy->check_command); + BUG_ON(curproxy->email_alert.flags & PR_EMAIL_ALERT_RESOLVED); if (defproxy->email_alert.mailers.name) curproxy->email_alert.mailers.name = strdup(defproxy->email_alert.mailers.name); if (defproxy->email_alert.from) @@ -1800,7 +1802,7 @@ static int proxy_defproxy_cpy(struct proxy *curproxy, const struct proxy *defpro if (defproxy->email_alert.myhostname) curproxy->email_alert.myhostname = strdup(defproxy->email_alert.myhostname); curproxy->email_alert.level = defproxy->email_alert.level; - curproxy->email_alert.set = defproxy->email_alert.set; + curproxy->email_alert.flags = defproxy->email_alert.flags; return 0; }