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.
This commit is contained in:
Aurelien DARRAGON 2024-06-17 18:53:48 +02:00
parent ee8be55942
commit 9d312212df
6 changed files with 18 additions and 6 deletions

View File

@ -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;

View File

@ -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;

View File

@ -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) {

View File

@ -3067,7 +3067,7 @@ int check_config_validity()
}
}
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', "

View File

@ -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);

View File

@ -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;
}