]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: proxy: fix email-alert invalid free
authorAurelien DARRAGON <adarragon@haproxy.com>
Mon, 17 Jun 2024 16:07:22 +0000 (18:07 +0200)
committerAurelien DARRAGON <adarragon@haproxy.com>
Mon, 17 Jun 2024 17:37:24 +0000 (19:37 +0200)
In fa90a7d3 ("BUG/MINOR: proxy: fix email-alert leak on deinit()"), I
tried to fix email-alert deinit() leak the simple way by leveraging
existing free_email_alert() helper function which was already used for
freeing email alert settings used in a default section.

However, as described in GH #2608, there is a subtelty that makes
free_email_alert() not suitable for use from free_proxy().

Indeed, proxy 'mailers.name' hint shares the same memory space than the
pointer to the corresponding mailers section (once the proxy is resolved,
name hint is replaced by the pointer to the section). However, since both
values share the same space (through union), we have to take care of not
freeing `mailers.name` once init_email_alert() was called on the proxy.

Unfortunately, free_email_alert() isn't protected against that, causing
double free() during deinit when mailers section is referenced from
multiple proxy sections. Since there is no easy fix, and that the leak in
itself isn't a big deal (fa90a7d3 was simply an opportunistic fix rather
than a must-have given that the leak only occurs during deinit and not
during runtime), let's actually revert the fix to restore legacy behavior
and prevent deinit errors.

Thanks to @snetat for having reported the issue on Github as well as
providing relevant infos to pinpoint the bug.

It should be backported everywhere fa90a7d3 was backported.
[ada: for versions prior to 3.0, simply revert the offending commit using
'git revert' as proxy_free_common() first appears in 3.0]

src/proxy.c

index 3acfdf6f84a3fd8b31d7028f2503c852c79c0c5a..83d7436e31c542781526eb7812489c18cfc065e9 100644 (file)
@@ -253,8 +253,6 @@ 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)
@@ -1483,6 +1481,7 @@ 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 ?