getaddrinfo(3) / AI_ADDRCONFIG

Submitted by Rich Felker on July 14, 2018, 2:31 a.m.

Details

Message ID 20180714023134.GH1392@brightrain.aerifal.cx
State New
Series "getaddrinfo(3) / AI_ADDRCONFIG"
Headers show

Commit Message

Rich Felker July 14, 2018, 2:31 a.m.
On Thu, Jul 12, 2018 at 10:53:12PM -0400, Christopher Friedt wrote:
> On Thu, Jul 12, 2018 at 9:49 PM Rich Felker <dalias@libc.org> wrote:
> > 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.
> 
> I originally wrote my patch with the intention of being as unobtrusive
> as possible but rather than disagree realized it was better to just do
> what you wanted me to.
> 
> The struct was a better solution for when the addrconfig logic lived
> in a separate function. It probably could have even been a separate
> static function inside of getaddrinfo.c, but I anticipated that you
> would not have liked that.
> 
> Definitely correct to disable pthread cancellation.
> 
> I used struct sockaddr_storage to avoid declaring more than one
> sockaddr because I thought you would have preferred that. Personally,
> I would have preferred to use two separate sockaddr too. Solves that
> problem.
> 
> Originally, I wanted to use a loop over the length of a table, but
> figured you would dislike that in favour of readability. Assuming
> there will only be ever be AF_INET and AF_INET6 support for
> getaddrinfo(3), handling it this or that way is fine.
> 
> The patch works for me as is or with the loop.

Here's a version with the loop. I've tested it now with ::1 removed
from device lo, but the connect to ::1 still succeeds; I suspect
presence of a default route for IPv6 makes it work since ::1 is
"routable" then. Can you confirm that it actually suppresses IPv6 in
your purely-no-IPv6 environment, as intended?

Rich

Patch hide | download patch | download mbox

diff --git a/src/network/getaddrinfo.c b/src/network/getaddrinfo.c
index b9439f7..91b4d30 100644
--- a/src/network/getaddrinfo.c
+++ b/src/network/getaddrinfo.c
@@ -3,6 +3,9 @@ 
 #include <netinet/in.h>
 #include <netdb.h>
 #include <string.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <endian.h>
 #include "lookup.h"
 
 int getaddrinfo(const char *restrict host, const char *restrict serv, const struct addrinfo *restrict hint, struct addrinfo **restrict res)
@@ -43,6 +46,35 @@  int getaddrinfo(const char *restrict host, const char *restrict serv, const stru
 		}
 	}
 
+	if (flags & AI_ADDRCONFIG) {
+		static const struct sockaddr_in lo4 = {
+			.sin_family = AF_INET, .sin_port = 65535,
+			.sin_addr.s_addr = __BYTE_ORDER == __BIG_ENDIAN
+				? 0x7f000001 : 0x0100007f
+		};
+		static const struct sockaddr_in6 lo6 = {
+			.sin6_family = AF_INET6, .sin6_port = 65535,
+			.sin6_addr = IN6ADDR_LOOPBACK_INIT
+		};
+		int tf[2] = { AF_INET, AF_INET6 };
+		const void *ta[2] = { &lo4, &lo6 };
+		socklen_t tl[2] = { sizeof lo4, sizeof lo6 };
+		int cs;
+		pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
+		for (i=0; i<2; i++) {
+			if (family==tf[1-i]) continue;
+			int s = socket(tf[i], SOCK_CLOEXEC|SOCK_DGRAM,
+				IPPROTO_UDP);
+			if (s<0) continue;
+			int r = connect(s, ta[i], tl[i]);
+			close(s);
+			if (!r) continue;
+			if (family == tf[i]) return EAI_NONAME;
+			family = tf[1-i];
+		}
+		pthread_setcancelstate(cs, 0);
+	}
+
 	nservs = __lookup_serv(ports, serv, proto, socktype, flags);
 	if (nservs < 0) return nservs;
 

Comments

Christopher Friedt July 14, 2018, 11:53 p.m.
On Fri, Jul 13, 2018 at 10:31 PM Rich Felker <dalias@libc.org> wrote:
> Here's a version with the loop. I've tested it now with ::1 removed
> from device lo, but the connect to ::1 still succeeds; I suspect
> presence of a default route for IPv6 makes it work since ::1 is
> "routable" then. Can you confirm that it actually suppresses IPv6 in
> your purely-no-IPv6 environment, as intended?

Works for me.
Rich Felker July 15, 2018, 12:07 a.m.
On Sat, Jul 14, 2018 at 07:53:53PM -0400, Christopher Friedt wrote:
> On Fri, Jul 13, 2018 at 10:31 PM Rich Felker <dalias@libc.org> wrote:
> > Here's a version with the loop. I've tested it now with ::1 removed
> > from device lo, but the connect to ::1 still succeeds; I suspect
> > presence of a default route for IPv6 makes it work since ::1 is
> > "routable" then. Can you confirm that it actually suppresses IPv6 in
> > your purely-no-IPv6 environment, as intended?
> 
> Works for me.

Thanks!

Rich
Rich Felker July 15, 2018, 12:19 a.m.
On Sat, Jul 14, 2018 at 08:07:17PM -0400, Rich Felker wrote:
> On Sat, Jul 14, 2018 at 07:53:53PM -0400, Christopher Friedt wrote:
> > On Fri, Jul 13, 2018 at 10:31 PM Rich Felker <dalias@libc.org> wrote:
> > > Here's a version with the loop. I've tested it now with ::1 removed
> > > from device lo, but the connect to ::1 still succeeds; I suspect
> > > presence of a default route for IPv6 makes it work since ::1 is
> > > "routable" then. Can you confirm that it actually suppresses IPv6 in
> > > your purely-no-IPv6 environment, as intended?
> > 
> > Works for me.
> 
> Thanks!

Minor issue: as written the patch left the cancel state disabled if it
exited early via return EAI_NONAME. Un-hoisting the setcancelstate
calls to fix.

Rich
Rich Felker July 15, 2018, 12:52 a.m.
On Sat, Jul 14, 2018 at 08:19:50PM -0400, Rich Felker wrote:
> On Sat, Jul 14, 2018 at 08:07:17PM -0400, Rich Felker wrote:
> > On Sat, Jul 14, 2018 at 07:53:53PM -0400, Christopher Friedt wrote:
> > > On Fri, Jul 13, 2018 at 10:31 PM Rich Felker <dalias@libc.org> wrote:
> > > > Here's a version with the loop. I've tested it now with ::1 removed
> > > > from device lo, but the connect to ::1 still succeeds; I suspect
> > > > presence of a default route for IPv6 makes it work since ::1 is
> > > > "routable" then. Can you confirm that it actually suppresses IPv6 in
> > > > your purely-no-IPv6 environment, as intended?
> > > 
> > > Works for me.
> > 
> > Thanks!
> 
> Minor issue: as written the patch left the cancel state disabled if it
> exited early via return EAI_NONAME. Un-hoisting the setcancelstate
> calls to fix.

And a couple other issues: socket() may fail with EAFNOSUPPORT, and in
that case, the family needs to be rejected rather than accepted like
it was with the continue. For other socket() failures (like
EMFILE/ENFILE), the result is indeterminate and we need to return
EAI_SYSTEM rather than wrong results. Making these changes and I think
it will be ready to commit.

Rich
Christopher Friedt July 15, 2018, 1:17 a.m.
On Sat, Jul 14, 2018, 8:53 PM Rich Felker, <dalias@libc.org> wrote:

> And a couple other issues: socket() may fail with EAFNOSUPPORT, and in
> that case, the family needs to be rejected rather than accepted like
> it was with the continue. For other socket() failures (like
> EMFILE/ENFILE), the result is indeterminate and we need to return
> EAI_SYSTEM rather than wrong results. Making these changes and I think
> it will be ready to commit.
>

Very thorough!

>
Christopher Friedt July 19, 2018, 12:14 a.m.
I could probably give it another spin with the suggestions you made if
you're preoccupied.
Rich Felker July 19, 2018, 12:49 a.m.
On Wed, Jul 18, 2018 at 08:14:22PM -0400, Christopher Friedt wrote:
> I could probably give it another spin with the suggestions you made if
> you're preoccupied.

Commit 187bcc3bf40bf187c5d76d206b04028fa8ca403b is upstream now based
on what we discussed. Let me know if you have any problems with it
still.

Rich
Christopher Friedt July 19, 2018, 12:57 a.m.
Awesome. I'll give it a go.