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 0fa3197a97149d19155bc3c6a74613da75dadedb..5df95a977de67d6c8b4fdca84e7bfae3c1b55a41 100644 (file)
--- a/include/haproxy/mailers-t.h
+++ b/include/haproxy/mailers-t.h
@@ -35,6 +35,13 @@
 #include <haproxy/tcpcheck-t.h>
 #include <haproxy/thread-t.h>
 
+/* 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 f6ed211c14c102eb9c97649c9ca0566689de62f0..262dc11d5e381f1e567fb220edd81ffb68b768f8 100644 (file)
--- 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 9ee81742c0c94c253990ad4e4bf4d19f24260c89..1f6ddc2adc898045f60622982250a3a67e197b2c 100644 (file)
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -22,6 +22,7 @@
 #include <haproxy/http_htx.h>
 #include <haproxy/http_ext.h>
 #include <haproxy/http_rules.h>
+#include <haproxy/mailers-t.h>
 #include <haproxy/listener.h>
 #include <haproxy/log.h>
 #include <haproxy/peers.h>
@@ -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 f7f64818832103117dd62c30a51e077519c6ec41..2434de2cb6c084f51643727a9dfb950c4c654954 100644 (file)
--- 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 85c18eebd52dd0d5b2cd4fd8ef0527b07c26b422..905bb6745a78619cf3043d15f394d9d6ec3311b1 100644 (file)
--- 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 a29f8a526170c2cd83308545239cb87773839ba7..10cb78025736802228fbfde8fef29be15f8ee186 100644 (file)
--- 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;
 }