From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13032 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: getaddrinfo(3) / AI_ADDRCONFIG Date: Thu, 12 Jul 2018 21:49:10 -0400 Message-ID: <20180713014910.GC1392@brightrain.aerifal.cx> References: <20180711003816.GV1392@brightrain.aerifal.cx> <20180711012640.GW1392@brightrain.aerifal.cx> <20180711164417.GX1392@brightrain.aerifal.cx> <20180711170034.GY1392@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="vs0rQTeTompTJjtd" X-Trace: blaine.gmane.org 1531446440 11266 195.159.176.226 (13 Jul 2018 01:47:20 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 13 Jul 2018 01:47:20 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-13048-gllmg-musl=m.gmane.org@lists.openwall.com Fri Jul 13 03:47:16 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1fdnAx-0002ou-I1 for gllmg-musl@m.gmane.org; Fri, 13 Jul 2018 03:47:15 +0200 Original-Received: (qmail 17958 invoked by uid 550); 13 Jul 2018 01:49:23 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 17932 invoked from network); 13 Jul 2018 01:49:22 -0000 Content-Disposition: inline In-Reply-To: Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:13032 Archived-At: --vs0rQTeTompTJjtd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Jul 11, 2018 at 09:20:20PM -0400, Christopher Friedt wrote: > On Wed, Jul 11, 2018 at 1:00 PM Rich Felker wrote: > > It could probably go inside __lookup_name, but maybe in getaddrinfo is > > better since that would avoid linking it for gethostbyname, etc. > > (which don't need it). > > Done > > > > inconsistent with musl (spaces > > > > after opening and before closing paren, etc.). > > Done > > > I think the one mandated by POSIX is EAI_NONAME ("The name does not > > resolve for the supplied parameters"). > > Done Something's not going right with our communication about this. I've attached an untested patch that's closer to what I've been looking for. It corrects an oversight I'd made, that we need to block cancellation during the probe, and localizes the change as originally requested. Please let me know if it works. Arguably it might be nicer to replace the repeated code with a table and 2-iteration for loop. Also: > diff --git a/src/network/getaddrinfo.c b/src/network/getaddrinfo.c > index b9439f77..72276ddc 100644 > --- a/src/network/getaddrinfo.c > +++ b/src/network/getaddrinfo.c > @@ -1,8 +1,10 @@ > +#include > #include > #include > #include > #include > #include > +#include > #include "lookup.h" > > int getaddrinfo(const char *restrict host, const char *restrict serv, const struct addrinfo *restrict hint, struct addrinfo **restrict res) > @@ -10,7 +12,7 @@ int getaddrinfo(const char *restrict host, const char *restrict serv, const stru > struct service ports[MAXSERVS]; > struct address addrs[MAXADDRS]; > char canon[256], *outcanon; > - int nservs, naddrs, nais, canon_len, i, j, k; > + int nservs, naddrs, nais, canon_len, i, j, k, fd, r; > int family = AF_UNSPEC, flags = 0, proto = 0, socktype = 0; > struct aibuf { > struct addrinfo ai; > @@ -19,6 +21,11 @@ int getaddrinfo(const char *restrict host, const char *restrict serv, const stru > struct sockaddr_in6 sin6; > } sa; > } *out; > + struct addrconfig { > + bool af_inet; > + bool af_inet6; > + } addrconfig; This struct, and use of bool, is completely gratuitous. > + struct sockaddr_storage sas; Use of sockaddr_storage is pretty much always a bug. > if (!host && !serv) return EAI_NONAME; > > @@ -33,6 +40,35 @@ int getaddrinfo(const char *restrict host, const char *restrict serv, const stru > if ((flags & mask) != flags) > return EAI_BADFLAGS; > > + if (flags & AI_ADDRCONFIG) { > + *((struct sockaddr_in *)(&sas)) = (struct sockaddr_in){ > + .sin_family = AF_INET, > + .sin_port = htons(42), > + .sin_addr.s_addr = INADDR_LOOPBACK, > + }; And indeed here undefined behavior is invoked by storing with an lvalue whose type does not match the type of the object. > + addrconfig.af_inet = false; > + r = socket(AF_INET, SOCK_DGRAM, 0); > + if (-1 != r) { > + fd = r; > + r = connect(fd, (struct sockaddr *) & sas, sizeof( struct sockaddr_in )); > + addrconfig.af_inet = 0 == r; > + close(fd); > + } > + *((struct sockaddr_in6 *)(&sas)) = (struct sockaddr_in6){ > + .sin6_family = AF_INET6, > + .sin6_port = htons(42), > + .sin6_addr = IN6ADDR_LOOPBACK_INIT, > + }; > + addrconfig.af_inet6 = false; > + r = socket(AF_INET6, SOCK_DGRAM, 0); > + if (-1 != r) { > + fd = r; > + r = connect(fd, (struct sockaddr *) & sas, sizeof(struct sockaddr_in6)); > + addrconfig.af_inet6 = 0 == r; > + close(fd); > + } > + } > + > switch (family) { > case AF_INET: > case AF_INET6: > @@ -61,30 +97,43 @@ int getaddrinfo(const char *restrict host, const char *restrict serv, const stru > outcanon = 0; > } > > - for (k=i=0; i - out[k].ai = (struct addrinfo){ > - .ai_family = addrs[i].family, > - .ai_socktype = ports[j].socktype, > - .ai_protocol = ports[j].proto, > - .ai_addrlen = addrs[i].family == AF_INET > - ? sizeof(struct sockaddr_in) > - : sizeof(struct sockaddr_in6), > - .ai_addr = (void *)&out[k].sa, > - .ai_canonname = outcanon, > - .ai_next = &out[k+1].ai }; > + for (k=i=0; i switch (addrs[i].family) { > case AF_INET: > + if ((flags & AI_ADDRCONFIG) && !addrconfig.af_inet) { > + nais--; > + continue; > + } > out[k].sa.sin.sin_family = AF_INET; > out[k].sa.sin.sin_port = htons(ports[j].port); > memcpy(&out[k].sa.sin.sin_addr, &addrs[i].addr, 4); > break; > case AF_INET6: > + if ((flags & AI_ADDRCONFIG ) && !addrconfig.af_inet6) { > + nais--; > + continue; > + } > out[k].sa.sin6.sin6_family = AF_INET6; > out[k].sa.sin6.sin6_port = htons(ports[j].port); > out[k].sa.sin6.sin6_scope_id = addrs[i].scopeid; > memcpy(&out[k].sa.sin6.sin6_addr, &addrs[i].addr, 16); > - break; > + break; > } > + out[k].ai = (struct addrinfo){ > + .ai_family = addrs[i].family, > + .ai_socktype = ports[j].socktype, > + .ai_protocol = ports[j].proto, > + .ai_addrlen = addrs[i].family == AF_INET > + ? sizeof(struct sockaddr_in) > + : sizeof(struct sockaddr_in6), > + .ai_addr = (void *)&out[k].sa, > + .ai_canonname = outcanon, > + .ai_next = &out[k+1].ai }; > + k++; > + } > + if ( nais < 1 ) { > + free( out ); > + return EAI_NONAME; > } > out[nais-1].ai.ai_next = 0; > *res = &out->ai; All of this is unnecessary, and fails the main legitimate purpose of AI_ADDRCONFIG, which is suppressing DNS queries that are known not to be needed. Instead family should simply be remapped before calling __lookup_name, as I suggested a few emails back in this thread. Rich --vs0rQTeTompTJjtd Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="ai_addrconfig.diff" diff --git a/src/network/getaddrinfo.c b/src/network/getaddrinfo.c index b9439f7..664a72a 100644 --- a/src/network/getaddrinfo.c +++ b/src/network/getaddrinfo.c @@ -3,6 +3,8 @@ #include #include #include +#include +#include #include "lookup.h" int getaddrinfo(const char *restrict host, const char *restrict serv, const struct addrinfo *restrict hint, struct addrinfo **restrict res) @@ -43,6 +45,46 @@ int getaddrinfo(const char *restrict host, const char *restrict serv, const stru } } + if (flags & AI_ADDRCONFIG) { + int cs; + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs); + if (family != AF_INET6) { + struct sockaddr_in sa4 = { + .sin_family = AF_INET, + .sin_port = 65535, + .sin_addr.s_addr = INADDR_LOOPBACK + }; + int s = socket(AF_INET, SOCK_CLOEXEC|SOCK_DGRAM, + IPPROTO_UDP); + if (s>=0) { + int r = connect(s, (void *)&sa4, sizeof sa4); + close(s); + if (r) { + if (family == AF_INET) return EAI_NONAME; + family = AF_INET6; + } + } + } + if (family != AF_INET) { + struct sockaddr_in6 sa6 = { + .sin6_family = AF_INET6, + .sin6_port = 65535, + .sin6_addr = IN6ADDR_LOOPBACK_INIT + }; + int s = socket(AF_INET6, SOCK_CLOEXEC|SOCK_DGRAM, + IPPROTO_UDP); + if (s>=0) { + int r = connect(s, (void *)&sa6, sizeof sa6); + close(s); + if (r) { + if (family == AF_INET6) return EAI_NONAME; + family = AF_INET; + } + } + } + pthread_setcancelstate(cs, 0); + } + nservs = __lookup_serv(ports, serv, proto, socktype, flags); if (nservs < 0) return nservs; --vs0rQTeTompTJjtd--