fix for usr.sbin/ikectl/parser.c

ok?

diff --git usr.sbin/ikectl/parser.c usr.sbin/ikectl/parser.c
index 52488845fd3..32099bb3b3d 100644
--- usr.sbin/ikectl/parser.c
+++ usr.sbin/ikectl/parser.c
@@ -273,6 +273,7 @@ parse_addr(const char *word)
        hints.ai_family = PF_UNSPEC;
        hints.ai_flags = AI_NUMERICHOST;
        if (getaddrinfo(word, "0", &hints, &r) == 0) {
+               freeaddrinfo(r);
                return (0);
        }
 
@@ -327,7 +328,6 @@ match_token(char *word, const struct token table[])
                case ADDRESS:
                case FQDN:
                        if (!match && word != NULL && strlen(word) > 0) {
-                               parse_addr(word);
                                res.host = strdup(word);
                                if (parse_addr(word) == 0)
                                        res.htype = HOST_IPADDR;


Thomas Barabosch([email protected]) on 2018.06.15 10:52:18 
+0200:
> Hi there,
> 
> yesterday I stumbled upon a memory leak in route6d, which got fixed. I
> was curious if I could find similar leaks. Please correct me if I am
> wrong but I suppose static analysis tools like coverity or clang
> analyzer do not detect this kind of bug since they do not know anything
> about the semantics of getaddrinfo/freeaddrinfo.
> 
> So I wrote a small script to check all .c-files that call getaddrinfo
> but do not call freeaddrinfo. Note that some of them may be false
> positives, i.e. the addrinfo struct is actually free'd in another source
> file. I had a look at some of the cases (but not all!). I've appended
> the output to the end of this email.
> 
> usr.bin/ssh/servconf.c
> 
> I think that a leak in usr.bin/ssh/servconf.c should be plugged. In
> function add_one_listen_addr, there is a call to getaddrinfo. This
> function is called in a loop in function add_listen_addr. So in this
> case that should leak memory depending on the number of loop iterations,
> right?
> 
> --------------------------------------------------------------------------------
> 
> libexec/mail.local/mail.local.c
> 
> In the case of mail.local, you may argument that this program terminates
> rather quickly and the memory is free'd by the kernel anyways. But it
> would be easy to fix.
> 
> --------------------------------------------------------------------------------
> 
> openbsd/usr.sbin/ndp/ndp.c
> 
> Same as mail.local, easy to fix.
> 
> --------------------------------------------------------------------------------
> 
> usr.sbin/npppd/common/net_utils.c
> 
> Seems to be a false positive.
> 
> --------------------------------------------------------------------------------
> 
> usr.sbin/smtpd/smtpc.c
> 
> Static variable, should be free'd once the program terminates.
> 
> --------------------------------------------------------------------------------
> 
> usr.sbin/ikectl/parser.c
> 
> getaddrinfo is called in function parse_addr (line 267). This function
> is called by function match_token (line 284), which happens in a
> for-loop. In this for loop, there is a case-matching. If the token
> matches ADDRESS or FQDN, parse_addr is called twice each time. So it
> leaks depending on the number of ADDRESS + FQDN tokens multiplied by two
> times. 
> 
> --------------------------------------------------------------------------------
> 
> usr.sbin/radiusd/util.c
> 
> False positive. getaddrinfo is called in util function addrport_parse
> and the addrinfo struct is returned by this function. However, it seems
> to be properly free'd in radiusd_radius.c.
> 
> --------------------------------------------------------------------------------
> 
> Could you please have a look at these cases? May be I am missing
> something here! Please note that the above method is very raw. It just
> greps for strings, there is no (sophisticated) static analysis involved
> similar to modules for malloc/free issue detection. Nevertheless, I
> thought that this may be of interest to you.
> 
> Best regards,
> 
> Thomas
> 
> ##########################################################################
> 
> OUTPUT:
> 
> openbsd/libexec/mail.local/mail.local.c
> 302: error = getaddrinfo("localhost", "biff", &hints, &res0);
> --------------------------------------------------------------------------------
> openbsd/usr.sbin/smtpd/smtpc.c
> 249: error = getaddrinfo(host, port, &hints, &res0);
> --------------------------------------------------------------------------------
> openbsd/usr.sbin/ikectl/parser.c
> 275: if (getaddrinfo(word, "0", &hints, &r) == 0) {
> --------------------------------------------------------------------------------
> openbsd/usr.sbin/radiusd/util.c
> 74: return (getaddrinfo(nodep, servp, &hints, p_ai));
> --------------------------------------------------------------------------------
> openbsd/usr.sbin/ndp/ndp.c
> 342: gai_error = getaddrinfo(host, NULL, &hints, &res);
> 416: gai_error = getaddrinfo(host, NULL, &hints, &res);
> 458: gai_error = getaddrinfo(host, NULL, &hints, &res);
> --------------------------------------------------------------------------------
> openbsd/usr.sbin/npppd/common/net_utils.c
> 131: return getaddrinfo(nodep, servp, &hints, p_ai);
> --------------------------------------------------------------------------------
> openbsd/usr.sbin/nsd/nsd.c
> 824: if ((r=getaddrinfo(node, (service?service:udp_port), &hints[h],
> &nsd.udp[i].addr)) != 0) {
> 839: if ((r=getaddrinfo(node, (service?service:tcp_port), &hints[h],
> &nsd.tcp[i].addr)) != 0) {
> --------------------------------------------------------------------------------
> openbsd/lib/libpcap/nametoaddr.c
> 101: error = getaddrinfo(name, NULL, &hints, &res);
> --------------------------------------------------------------------------------
> openbsd/lib/libc/asr/getaddrinfo.c
> 28: getaddrinfo(const char *hostname, const char *servname,
> --------------------------------------------------------------------------------
> openbsd/usr.bin/ssh/servconf.c
> 693: if ((gaierr = getaddrinfo(addr, strport, &hints, &aitop)) != 0)
> --------------------------------------------------------------------------------
> openbsd/regress/lib/libc/getaddrinfo/gaitest.c
> 180: error = getaddrinfo(p, q, &ai, &res);
> --------------------------------------------------------------------------------
> openbsd/regress/sys/netinet6/pktinfo_addr/runtest.c
> 96: rc = getaddrinfo(argv[optind], PORTNUM, &hints, &in6ai);
> 98: errx(2, "getaddrinfo(%s) = %d: %s",
> --------------------------------------------------------------------------------
> openbsd/regress/sys/net/pflow/gen_traffic.c
> 94: error = getaddrinfo(ip, "12346", &hints, &server_res);
> 115: error = getaddrinfo(ip, "12345", &hints, &sender_res);
> --------------------------------------------------------------------------------
> openbsd/regress/sys/netinet/in_pcbbind/runtest.c
> 367: if ((error = getaddrinfo(baddr_s, bport_s, &hints, &baddr)))
> 368: errx(2, "getaddrinfo(%s,%s): %s", baddr_s, bport_s,
> 373: if ((error = getaddrinfo(NULL, bport_s, &hints, &any)))
> 374: errx(2, "getaddrinfo(NULL,%s): %s", bport_s,
> 388: if ((error = getaddrinfo(bmifa_s, NULL, &hints, &mifa)))
> 389: errx(2, "getaddrinfo(%s,NULL): %s", bmifa_s,
> 410: if ((error = getaddrinfo(bmifa_s, NULL, &hints, &mifa)))
> 411: errx(2, "getaddrinfo(%s,NULL): %s", bmifa_s,
> --------------------------------------------------------------------------------
> openbsd/regress/sys/netinet/sendsrcaddr/runtest.c
> 73: error = getaddrinfo(optarg, NULL, &hints, &res);
> 79: error = getaddrinfo(optarg, NULL, &hints, &res);
> 85: error = getaddrinfo(optarg, NULL, &hints, &res);
> 100: error = getaddrinfo(optarg, NULL, &hints, &res);
> 106: error = getaddrinfo(optarg, NULL, &hints, &res);
> --------------------------------------------------------------------------------
> 
> -- 
> Thomas Barabosch
> 
> Fraunhofer FKIE                        Tel.:   +49 228 50212-601
> Cyber Analysis & Defense               Fax:    +49 228 73-4571
> Zanderstra??e 5                         D-53113 Bonn, Germany  
> http://www.fkie.fraunhofer.de/
> 
> 


Reply via email to