Hi

attached is a patch that fixes a memory leak in str2ip2. I wasn't sure
about the severity of this bug (it's only 140 Bytes per call for me) and
opted for MEDIUM. Change if you think MAJOR (?) is more warranted for a
memory leak.

Also I wasn't sure how I would structure the code best. I did not want to
copy the freeaddrinfo into every case, because don't repeat yourself, but
I am not sure whether the `success` variable is any better.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --
Subject: [PATCH] BUG/MEDIUM: standard: Fix memory leak in str2ip2()

An haproxy compiled with:

> make -j4 all TARGET=linux2628 USE_GETADDRINFO=1

And running with a configuration like this:

  defaults
        log     global
        mode    http
        option  httplog
        option  dontlognull
        timeout connect 5000
        timeout client  50000
        timeout server  50000

  frontend fe
        bind :::8080 v4v6

        default_backend be

  backend be
        server s example.com:80 check

Will leak memory inside `str2ip2()`, because the list `result` is not
properly freed in success cases:

==18875== 140 (76 direct, 64 indirect) bytes in 1 blocks are definitely lost in 
loss record 87 of 111
==18875==    at 0x4C2DB8F: malloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18875==    by 0x537A565: gaih_inet (getaddrinfo.c:1223)
==18875==    by 0x537DD5D: getaddrinfo (getaddrinfo.c:2425)
==18875==    by 0x4868E5: str2ip2 (standard.c:733)
==18875==    by 0x43F28B: srv_set_addr_via_libc (server.c:3767)
==18875==    by 0x43F50A: srv_iterate_initaddr (server.c:3879)
==18875==    by 0x43F50A: srv_init_addr (server.c:3944)
==18875==    by 0x475B30: init (haproxy.c:1595)
==18875==    by 0x40406D: main (haproxy.c:2479)

The exists as long as the usage of getaddrinfo in that function exists,
it was introduced in commit:
d5f4328efd5f4eaa7c89cad9773124959195430a

v1.5-dev8 is the first tag containing this comment, the fix
should be backported to haproxy 1.5 and newer.
---
 src/standard.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/standard.c b/src/standard.c
index acd136c27..4f89a9217 100644
--- a/src/standard.c
+++ b/src/standard.c
@@ -722,6 +722,7 @@ struct sockaddr_storage *str2ip2(const char *str, struct 
sockaddr_storage *sa, i
 #ifdef USE_GETADDRINFO
        if (global.tune.options & GTUNE_USE_GAI) {
                struct addrinfo hints, *result;
+               int success = 0;
 
                memset(&result, 0, sizeof(result));
                memset(&hints, 0, sizeof(hints));
@@ -733,23 +734,30 @@ struct sockaddr_storage *str2ip2(const char *str, struct 
sockaddr_storage *sa, i
                if (getaddrinfo(str, NULL, &hints, &result) == 0) {
                        if (!sa->ss_family || sa->ss_family == AF_UNSPEC)
                                sa->ss_family = result->ai_family;
-                       else if (sa->ss_family != result->ai_family)
+                       else if (sa->ss_family != result->ai_family) {
+                               freeaddrinfo(result);
                                goto fail;
+                       }
 
                        switch (result->ai_family) {
                        case AF_INET:
                                memcpy((struct sockaddr_in *)sa, 
result->ai_addr, result->ai_addrlen);
                                set_host_port(sa, port);
-                               return sa;
+                               success = 1;
+                               break;
                        case AF_INET6:
                                memcpy((struct sockaddr_in6 *)sa, 
result->ai_addr, result->ai_addrlen);
                                set_host_port(sa, port);
-                               return sa;
+                               success = 1;
+                               break;
                        }
                }
 
                if (result)
                        freeaddrinfo(result);
+
+               if (success)
+                       return sa;
        }
 #endif
        /* try to resolve an IPv4/IPv6 hostname */
-- 
2.15.1


Reply via email to