Hi there,
I appended a patch for ndp.c.
Best regards,
Thomas
Am 16.06.18 um 11:23 schrieb Sebastian Benoit:
> 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/
>>
>>
>
--
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/
--- usr.sbin/ndp/ndp.c 2018-06-14 20:45:59.099429325 +0200
+++ usr.sbin/ndp/ndp.c.patched 2018-06-16 13:41:27.722180811 +0200
@@ -352,6 +352,7 @@
htons(((struct sockaddr_in6 *)res->ai_addr)->sin6_scope_id);
}
#endif
+ freeaddrinfo(res);
ea = (u_char *)LLADDR(&sdl_m);
if (ndp_ether_aton(eaddr, ea) == 0)
sdl_m.sdl_alen = 6;
@@ -426,6 +427,7 @@
htons(((struct sockaddr_in6 *)res->ai_addr)->sin6_scope_id);
}
#endif
+ freeaddrinfo(res);
dump(&sin->sin6_addr, 0);
if (found_entry == 0) {
getnameinfo((struct sockaddr *)sin, sin->sin6_len, host_buf,
@@ -468,7 +470,7 @@
htons(((struct sockaddr_in6 *)res->ai_addr)->sin6_scope_id);
}
#endif
-
+ freeaddrinfo(res);
if (rtget(&sin, &sdl)) {
errx(1, "RTM_GET(%s) failed", host);
/* NOTREACHED */
