Re: [patch] Re: Possible sasyncd memory leak ?

[Michał Koc]

W dniu 21.03.2019 o 07:21, Otto Moerbeek pisze:

On Thu, Mar 21, 2019 at 12:51:11AM +0100, Klemens Nanni wrote:

On Tue, Mar 12, 2019 at 03:19:56PM +0100, Otto Moerbeek wrote:

I also fixed a case of parsing IPv6 addresses.

Anyone willing to ok?

See comments inline.

And now also with a lexer bug fixed.  Earlier I thougt it was an order
dependency in the clauses. But is was an order dependency in comment
lines and empty lines.
+check_peer_addr(const char *peer_addr)
+{
+       int valid = 1;
+       short peer_family = AF_UNSPEC;
+       struct ifaddrs *ifap = NULL, *ifa;
+       struct syncpeer *peer;
+       struct sockaddr_in sa;
+       struct sockaddr_in6 sa6;
+
+       if (inet_pton(AF_INET, peer_addr, &sa.sin_addr) == 1)
+               peer_family = AF_INET;
+
+       if (peer_family == AF_UNSPEC && inet_pton(AF_INET6, peer_addr,
+           &sa6.sin6_addr) == 1)
+               peer_family = AF_INET6;

`peer_addr' must not be a CIDR network, so I'd go with the more AF
agnostic getaddrinfo(3) and check for res->ai_family in any case...

+       if (peer_family == AF_UNSPEC) {
+               log_msg(2, "config: skip unparseable peer %s", peer_addr);
+               valid = 0;
+       }

..but `peer_addr' may also be a hostname, so that is not handled by
your diff:

net.h:  char            *name;          /* FQDN or an IP, from conf */

getaddrinfo(3) can resolve however, thus inet_pton(3) should not be used
here.

Either way, this is a job for host_ip() as found in pfctl or bgpd.
Other daemons like iked still have host_v4() and host_v6().  Parsers use
these functions to check for valid addresses, so I'd say sasyncd should
be no exception and adopt the same approach.

@@ -325,7 +386,7 @@ yylex(void)
        /* Numerical token? */
        if (isdigit(*confptr)) {
                for (p = confptr; *p; p++)
-                       if (*p == '.') /* IP address, or bad input */
+                       if (*p == '.' || *p == ':') /* IP address, or bad input 
*/

This fixes the parser as in

        # echo peer 2001:db8::1 > conf
        # sasyncd -dnv -c conf
        config: syntax error
        # ./obj/sasyncd -dnv -c conf
        configuration OK

But I have not actually tested this with a proper IPv6 setup since I do
not use sasyncd; did you?

@@ -397,6 +458,9 @@ conf_parse_file(char *cfgfile)
                if (*s == '#') {
                        while (*s != '\n' && s < buf + conflen)
                                s++;
+                       while (*s == '\n' && s < buf + conflen)
+                               s++;
+                       s--;

OK kn for this fix alone.

I'll test an ipv6 seup and commit the lexer parts if those work.
Michal, can you work on the first set of comments? I think Klemens is
right.

        -Otto

Of course, I'll follow the comments soon

Best Regards
M.K.