BUG/MINOR: resolvers: fix leaked fields on cfg_parse_resolvers() error paths

cfg_parse_resolvers() has many error paths on allocation failure when
parsing "nameserver". These paths handle their own cleanup instead of
centralizing it. The result is that some errors paths leak some fields.
The most complex ones are the strdup() failures which require to check
for stream or dgram to figure what to free. These can be detected via
ASAN on a dummy strdup() allocation failure:

  Indirect leak of 131080 byte(s) in 1 object(s) allocated from:
      #0 0x7f0b7ed1f0ab in malloc (/usr/lib64/libasan.so.8+0x11f0ab)
      #1 0x000000c73e19 in dns_ring_new src/dns_ring.c:59
      #2 0x000000af1848 in dns_dgram_init src/dns.c:480
      #3 0x000000922005 in cfg_parse_resolvers src/resolvers.c:3792
      #4 0x00000089d33e in parse_cfg src/cfgparse.c:2202
      #5 0x0000009e0a39 in read_cfg src/haproxy.c:1142
      #6 0x000000447e8c in main src/haproxy.c:3474
      #7 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
      #8 0x7ffd35f1531c  ([stack]+0x2031c)

  Indirect leak of 304 byte(s) in 1 object(s) allocated from:
      #0 0x7f0b7ed1ea23 in calloc (/usr/lib64/libasan.so.8+0x11ea23)
      #1 0x000000af1681 in dns_dgram_init src/dns.c:468
      #2 0x000000922005 in cfg_parse_resolvers src/resolvers.c:3792
      #3 0x00000089d33e in parse_cfg src/cfgparse.c:2202
      #4 0x0000009e0a39 in read_cfg src/haproxy.c:1142
      #5 0x000000447e8c in main src/haproxy.c:3474
      #6 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
      #7 0x7ffd35f1531c  ([stack]+0x2031c)

  Indirect leak of 104 byte(s) in 1 object(s) allocated from:
      #0 0x7f0b7ed1ea23 in calloc (/usr/lib64/libasan.so.8+0x11ea23)
      #1 0x000000921f83 in cfg_parse_resolvers src/resolvers.c:3772
      #2 0x00000089d33e in parse_cfg src/cfgparse.c:2202
      #3 0x0000009e0a39 in read_cfg src/haproxy.c:1142
      #4 0x000000447e8c in main src/haproxy.c:3474
      #5 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
      #6 0x7ffd35f1531c  ([stack]+0x2031c)

  Indirect leak of 64 byte(s) in 1 object(s) allocated from:
      #0 0x7f0b7ed1f0ab in malloc (/usr/lib64/libasan.so.8+0x11f0ab)
      #1 0x000000c73e09 in dns_ring_new src/dns_ring.c:55
      #2 0x000000af1848 in dns_dgram_init src/dns.c:480
      #3 0x000000922005 in cfg_parse_resolvers src/resolvers.c:3792
      #4 0x00000089d33e in parse_cfg src/cfgparse.c:2202
      #5 0x0000009e0a39 in read_cfg src/haproxy.c:1142
      #6 0x000000447e8c in main src/haproxy.c:3474
      #7 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
      #8 0x7ffd35f1531c  ([stack]+0x2031c)

  Indirect leak of 15 byte(s) in 1 object(s) allocated from:
      #0 0x7f0b7ed18e20 in strdup (/usr/lib64/libasan.so.8+0x118e20)
      #1 0x00000092203b in cfg_parse_resolvers src/resolvers.c:3798
      #2 0x00000089d33e in parse_cfg src/cfgparse.c:2202
      #3 0x0000009e0a39 in read_cfg src/haproxy.c:1142
      #4 0x000000447e8c in main src/haproxy.c:3474
      #5 0x7f0b7e02ad13 in __libc_start_call_main (/lib64/libc.so.6+0x2ad13)
      #6 0x7ffd35f1531c  ([stack]+0x2031c)

This should be completely reworked so that the cleanup is performed in
a central place, as the risk to get it wrong remains high.

This patch does the minimal changes to clean this up. It does not need
to be backported since it only triggers on boot OOM.

diff --git a/src/resolvers.c b/src/resolvers.c
index 44a4ee3e2d2f0100018bdea680e3198f848853c3..5ae27547279305a9aa32f2a958fb11275bf81326 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -3780,30 +3780,56 @@ int cfg_parse_resolvers(const char *file, int linenum, char **args, int kwm)
                                                 SRV_PARSE_PARSE_ADDR|SRV_PARSE_INITIAL_RESOLVE);
                        if (err_code & (ERR_FATAL|ERR_ABORT)) {
                                err_code |= ERR_ABORT;
+                               free(newnameserver);
                                goto out;
                        }
 
                        if (dns_stream_init(newnameserver, curr_resolvers->px->srv) < 0) {
                                ha_alert("parsing [%s:%d] : out of memory.\n", file, linenum);
                                err_code |= ERR_ALERT|ERR_ABORT;
+                               free(newnameserver);
                                goto out;
                        }
                }
                else if (dns_dgram_init(newnameserver, sk) < 0) {
                        ha_alert("parsing [%s:%d] : out of memory.\n", file, linenum);
                        err_code |= ERR_ALERT | ERR_ABORT;
+                       free(newnameserver);
                        goto out;
                }
 
                if ((newnameserver->conf.file = strdup(file)) == NULL) {
                        ha_alert("parsing [%s:%d] : out of memory.\n", file, linenum);
                        err_code |= ERR_ALERT | ERR_ABORT;
+                       if (newnameserver->stream) {
+                               dns_ring_free(newnameserver->stream->ring_req);
+                               task_destroy(newnameserver->stream->task_req);
+                               task_destroy(newnameserver->stream->task_rsp);
+                               task_destroy(newnameserver->stream->task_idle);
+                               free(newnameserver->stream);
+                       } else if (newnameserver->dgram) {
+                               dns_ring_free(newnameserver->dgram->ring_req);
+                               free(newnameserver->dgram);
+                       }
+                       free(newnameserver);
                        goto out;
                }
 
                if ((newnameserver->id = strdup(args[1])) == NULL) {
                        ha_alert("parsing [%s:%d] : out of memory.\n", file, linenum);
                        err_code |= ERR_ALERT | ERR_ABORT;
+                       free((char *)newnameserver->conf.file);
+                       if (newnameserver->stream) {
+                               dns_ring_free(newnameserver->stream->ring_req);
+                               task_destroy(newnameserver->stream->task_req);
+                               task_destroy(newnameserver->stream->task_rsp);
+                               task_destroy(newnameserver->stream->task_idle);
+                               free(newnameserver->stream);
+                       } else if (newnameserver->dgram) {
+                               dns_ring_free(newnameserver->dgram->ring_req);
+                               free(newnameserver->dgram);
+                       }
+                       free(newnameserver);
                        goto out;
                }