On Sun, Nov 30, 2003 at 03:45:17PM +0100, Matthieu Herrb wrote: >David Dawes wrote (in a message from Thursday 27) > > > > Incidentally related to this is another problem I've run in to. Old > > FreeBSD X apps, that use a version of libc without IPv6 support, crash > > when run on a recent system when trying to connect to a remote display > > when using our newly built X libraries. I have a patch that restores > > compatibility to our X libraries by checking if the IPv6 libc support > > is available at run-time, and falling back to the IPv4 code otherwise. > > I think it is "complete enough", but I'd like some feedback before > > committing it. > >I think that this particular case (using binary libraries with IPv6 >enabled on a older libc which doesn't know at all about IPv6) is >probably going to fail in applications too. I'm not sure if >it's worth handling it inside of Xlib. There are enough issues and >separate cases when the libraries are consistent wrt. IPv6 to handle >not to add this additional complexity. > >But having said that, your patch seem to handle this particular >situation and since it's the base for your 2nd patch that handles >something more important, I guess both should go in.
I agree that it isn't worth going to extreme lengths. But since it was relatively straightforward to do, I think it is worth doing to avoid breaking what is probably the most common usage. This essentially amounted to changing a build-time choice to a run-time choice, so didn't really require new code. The separation of IPv4 and IPv6 sockets/addresses required the most changes. I've attached an updated verion of the complete patch, including the EHOSTDOWN patch you posted today. It fixes a couple of flaws in the IPv4/IPv6 separation, and cleans up a few other things. (It is a 'diff -ubB' patch so whitespace noise is minimised.) I'm planning on committing it in a day or so pending further testing and feedback. Post-4.4 we might want to look at whether this (and other IPv6-related code) needs to be restructured to handle the separation more cleanly and naturally. I don't follow the IPv6 standards process closely, so I don't know how much of the separation vs combining is "political", or whether it is likely that future RFCs will favour one method over the other. If we handle IPv6 by keeping IPv6 sockets for IPv6 only, we should be in good shape either way. David -- David Dawes developer/release engineer The XFree86 Project www.XFree86.org/~dawes
--- Xtranssock.c-3.65 Sun Nov 30 19:15:30 2003 +++ Xtranssock.c Sun Nov 30 19:17:25 2003 @@ -206,6 +206,9 @@ #define NUMSOCKETFAMILIES (sizeof(Sockettrans2devtab)/sizeof(Sockettrans2dev)) +#ifdef TCPCONN +static int TRANS(SocketINETClose) (XtransConnInfo ciptr); +#endif #ifdef UNIXCONN @@ -278,6 +281,8 @@ #if defined(IPv6) && defined(AF_INET6) static const struct in6_addr local_in6addr_any = IN6ADDR_ANY_INIT; #pragma weak in6addr_any = local_in6addr_any +#pragma weak getaddrinfo +static int haveIPv6 = 1; #endif /* @@ -312,19 +317,32 @@ { #if defined(IPv6) && defined(AF_INET6) - struct sockaddr_storage sockname; -#else - struct sockaddr_in sockname; + struct sockaddr_storage socknamev6; #endif + struct sockaddr_in socknamev4; + void *socknamePtr; #if defined(SVR4) || defined(SCO325) - size_t namelen = sizeof sockname; + size_t namelen; #else - int namelen = sizeof sockname; + int namelen; #endif PRMSG (3,"SocketINETGetAddr(%p)\n", ciptr, 0, 0); - if (getsockname (ciptr->fd,(struct sockaddr *) &sockname, +#if defined(IPv6) && defined(AF_INET6) + if (haveIPv6) + { + namelen = sizeof(socknamev6); + socknamePtr = &socknamev6; + } + else +#endif + { + namelen = sizeof(socknamev4); + socknamePtr = &socknamev4; + } + + if (getsockname (ciptr->fd,(struct sockaddr *) socknamePtr, (void *)&namelen) < 0) { PRMSG (1,"SocketINETGetAddr: getsockname() failed: %d\n", @@ -345,12 +363,17 @@ } #if defined(IPv6) && defined(AF_INET6) - ciptr->family = ((struct sockaddr *)&sockname)->sa_family; -#else - ciptr->family = sockname.sin_family; + if (haveIPv6) + { + ciptr->family = ((struct sockaddr *)socknamePtr)->sa_family; + } + else #endif + { + ciptr->family = socknamev4.sin_family; + } ciptr->addrlen = namelen; - memcpy (ciptr->addr, &sockname, ciptr->addrlen); + memcpy (ciptr->addr, socknamePtr, ciptr->addrlen); return 0; } @@ -366,19 +389,32 @@ { #if defined(IPv6) && defined(AF_INET6) - struct sockaddr_storage sockname; -#else - struct sockaddr_in sockname; + struct sockaddr_storage socknamev6; #endif + struct sockaddr_in socknamev4; + void *socknamePtr; #if defined(SVR4) || defined(SCO325) - size_t namelen = sizeof sockname; + size_t namelen; #else - int namelen = sizeof sockname; + int namelen; #endif +#if defined(IPv6) && defined(AF_INET6) + if (haveIPv6 && ciptr->family == AF_INET6) + { + namelen = sizeof(socknamev6); + socknamePtr = &socknamev6; + } + else +#endif + { + namelen = sizeof(socknamev4); + socknamePtr = &socknamev4; + } + PRMSG (3,"SocketINETGetPeerAddr(%p)\n", ciptr, 0, 0); - if (getpeername (ciptr->fd, (struct sockaddr *) &sockname, + if (getpeername (ciptr->fd, (struct sockaddr *) socknamePtr, (void *)&namelen) < 0) { PRMSG (1,"SocketINETGetPeerAddr: getpeername() failed: %d\n", @@ -399,7 +435,7 @@ } ciptr->peeraddrlen = namelen; - memcpy (ciptr->peeraddr, &sockname, ciptr->peeraddrlen); + memcpy (ciptr->peeraddr, socknamePtr, ciptr->peeraddrlen); return 0; } @@ -413,6 +449,14 @@ PRMSG (3,"SocketOpen(%d,%d)\n", i, type, 0); +#if defined(IPv6) && defined(AF_INET6) + if (getaddrinfo == NULL) + haveIPv6 = 0; + + if (!haveIPv6 && Sockettrans2devtab[i].family == AF_INET6) + return NULL; +#endif + if ((ciptr = (XtransConnInfo) xcalloc ( 1, sizeof(struct _XtransConnInfo))) == NULL) { @@ -488,19 +532,18 @@ #ifdef TRANS_CLIENT static XtransConnInfo -TRANS(SocketOpenCOTSClient) (Xtransport *thistrans, char *protocol, - char *host, char *port) - +TRANS(SocketOpenCOTSClientBase) (char *transname, char *protocol, + char *host, char *port, int previndex) { XtransConnInfo ciptr; - int i = -1; + int i = previndex; PRMSG (2, "SocketOpenCOTSClient(%s,%s,%s)\n", protocol, host, port); SocketInitOnce(); - while ((i = TRANS(SocketSelectFamily) (i, thistrans->TransName)) >= 0) { + while ((i = TRANS(SocketSelectFamily) (i, transname)) >= 0) { if ((ciptr = TRANS(SocketOpen) ( i, Sockettrans2devtab[i].devcotsname)) != NULL) break; @@ -508,10 +551,10 @@ if (i < 0) { if (i == -1) PRMSG (1,"SocketOpenCOTSClient: Unable to open socket for %s\n", - thistrans->TransName, 0, 0); + transname, 0, 0); else PRMSG (1,"SocketOpenCOTSClient: Unable to determine socket type for %s\n", - thistrans->TransName, 0, 0); + transname, 0, 0); return NULL; } @@ -522,6 +565,15 @@ return ciptr; } +static XtransConnInfo +TRANS(SocketOpenCOTSClient) (Xtransport *thistrans, char *protocol, + char *host, char *port) +{ + return TRANS(SocketOpenCOTSClientBase)( + thistrans->TransName, protocol, host, port, -1); +} + + #endif /* TRANS_CLIENT */ @@ -1335,9 +1387,8 @@ #if defined(IPv6) && defined(AF_INET6) struct addrinfo hints; char ntopbuf[INET6_ADDRSTRLEN]; - struct sockaddr_in6 tmpsin6; int resetonce = 0; -#else +#endif struct sockaddr_in sockname; #ifdef XTHREADS_NEEDS_BYNAMEPARAMS _Xgethostbynameparams hparams; @@ -1346,14 +1397,11 @@ struct hostent *hostp; struct servent *servp; unsigned long tmpaddr; -#endif #ifdef X11_t char portbuf[PORTBUFSIZE]; #endif -#if defined(X11_t) || !defined(IPv6) || !defined(AF_INET6) long tmpport; -#endif char hostnamebuf[256]; /* tmp space */ PRMSG (2,"SocketINETConnect(%d,%s,%s)\n", ciptr->fd, host, port); @@ -1384,6 +1432,7 @@ #endif #if defined(IPv6) && defined(AF_INET6) + if (haveIPv6) { if (addrlist != NULL) { if (strcmp(host,addrlist->host) || strcmp(port,addrlist->port)) { if (addrlist->firstaddr) @@ -1406,13 +1455,13 @@ res = getaddrinfo(host,port,&hints,&addrlist->firstaddr); if (res != 0) { - PRMSG (1, - "TRANS(SocketINETConnect) () can't get address for %s:%s: %s\n", - host, port, gai_strerror(res)); + PRMSG (1, "SocketINETConnect() can't get address " + "for %s:%s: %s\n", host, port, gai_strerror(res)); ESET(EINVAL); return TRANS_CONNECT_FAILED; } - for (res = 0, addrlist->addr = addrlist->firstaddr; addrlist->addr ; res++) { + for (res = 0, addrlist->addr = addrlist->firstaddr; + addrlist->addr ; res++) { addrlist->addr = addrlist->addr->ai_next; } PRMSG(4,"Got New Address list with %d addresses\n", res, 0, 0); @@ -1420,13 +1469,12 @@ addrlist->addr = NULL; } - while ( socketaddr == NULL ) { + while (socketaddr == NULL) { if (addrlist->addr == NULL) { if (resetonce) { /* Already checked entire list - no usable addresses */ - PRMSG (1, - "TRANS(SocketINETConnect) () no usable address for %s:%s\n", - host, port, 0); + PRMSG (1, "SocketINETConnect() no usable address " + "for %s:%s\n", host, port, 0); return TRANS_CONNECT_FAILED; } else { /* Go back to beginning of list */ @@ -1442,48 +1489,83 @@ if (addrlist->addr->ai_family == AF_INET) { struct sockaddr_in *sin = (struct sockaddr_in *) socketaddr; - PRMSG (4,"TRANS(SocketINETConnect) sockname.sin_addr = %s\n", + PRMSG (4,"SocketINETConnect() sockname.sin_addr = %s\n", inet_ntop(addrlist->addr->ai_family,&sin->sin_addr, ntopbuf,sizeof(ntopbuf)), 0, 0); - PRMSG (4,"TRANS(SocketINETConnect) sockname.sin_port = %d\n", + PRMSG (4,"SocketINETConnect() sockname.sin_port = %d\n", ntohs(sin->sin_port), 0, 0); if (Sockettrans2devtab[ciptr->index].family == AF_INET6) { - /* We have IPv6 socket, need to map IPv4 address to IPv6 */ - char *v4addr = (char *) &sin->sin_addr.s_addr; - bzero(&tmpsin6, sizeof(tmpsin6)); -#ifdef SIN6_LEN - tmpsin6.sin6_len = sizeof(tmpsin6); -#endif - tmpsin6.sin6_family = AF_INET6; - tmpsin6.sin6_port = sin->sin_port; - tmpsin6.sin6_addr.s6_addr[10] = 0xff; - tmpsin6.sin6_addr.s6_addr[11] = 0xff; - tmpsin6.sin6_addr.s6_addr[12] = v4addr[0]; - tmpsin6.sin6_addr.s6_addr[13] = v4addr[1]; - tmpsin6.sin6_addr.s6_addr[14] = v4addr[2]; - tmpsin6.sin6_addr.s6_addr[15] = v4addr[3]; - - socketaddr = (struct sockaddr *) &tmpsin6; - socketaddrlen = sizeof(tmpsin6); - PRMSG (4,"TRANS(SocketINETConnect) sockname.sin6_addr = %s\n", - inet_ntop(AF_INET6, &tmpsin6.sin6_addr, ntopbuf, - sizeof(ntopbuf)), 0, 0); + if (strcmp(Sockettrans2devtab[ciptr->index].transname, + "tcp") == 0) { + XtransConnInfo newciptr; + + /* + * Our socket is an IPv6 socket, but the address is + * IPv4. Close it and get an IPv4 socket. This is + * needed for IPv4 connections to work on platforms + * that don't allow IPv4 over IPv6 sockets. + */ + TRANS(SocketINETClose)(ciptr); + newciptr = TRANS(SocketOpenCOTSClientBase)( + "tcp", "tcp", host, port, ciptr->index); + if (newciptr) { + ciptr->fd = newciptr->fd; + xfree(newciptr); + } + if (!newciptr || + Sockettrans2devtab[newciptr->index].family != + AF_INET) { + socketaddr = NULL; + PRMSG (4,"SocketINETConnect() Cannot get IPv4 " + " socketfor IPv4 address\n", 0,0,0); + } + } else { + socketaddr = NULL; + PRMSG (4,"SocketINETConnect Skipping IPv4 address\n", + 0,0,0); + } } } else if (addrlist->addr->ai_family == AF_INET6) { struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *) socketaddr; - PRMSG (4,"TRANS(SocketINETConnect) sockname.sin6_addr = %s\n", - inet_ntop(addrlist->addr->ai_family, &sin6->sin6_addr,ntopbuf,sizeof(ntopbuf)), + PRMSG (4,"SocketINETConnect() sockname.sin6_addr = %s\n", + inet_ntop(addrlist->addr->ai_family, + &sin6->sin6_addr,ntopbuf,sizeof(ntopbuf)), 0, 0); - PRMSG (4,"TRANS(SocketINETConnect) sockname.sin6_port = %d\n", + PRMSG (4,"SocketINETConnect() sockname.sin6_port = %d\n", ntohs(sin6->sin6_port), 0, 0); if (Sockettrans2devtab[ciptr->index].family == AF_INET) { - PRMSG (4,"TRANS(SocketINETConnect) Skipping IPv6 address\n", - 0,0,0); + if (strcmp(Sockettrans2devtab[ciptr->index].transname, + "tcp") == 0) { + XtransConnInfo newciptr; + + /* + * Close the IPv4 socket and try to open an IPv6 socket. + */ + TRANS(SocketINETClose)(ciptr); + newciptr = TRANS(SocketOpenCOTSClientBase)( + "tcp", "tcp", host, port, -1); + if (newciptr) { + ciptr->fd = newciptr->fd; + xfree(newciptr); + } + if (!newciptr || + Sockettrans2devtab[newciptr->index].family != + AF_INET6) { socketaddr = NULL; + PRMSG (4,"SocketINETConnect() Cannot get IPv6 " + "socket for IPv6 address\n", 0,0,0); + } + } + else + { + socketaddr = NULL; + PRMSG (4,"SocketINETConnect() Skipping IPv6 address\n", + 0,0,0); + } } } else { socketaddr = NULL; /* Unsupported address type */ @@ -1492,7 +1574,9 @@ addrlist->addr = addrlist->addr->ai_next; } } -#else + } else +#endif + { /* * Build the socket name. */ @@ -1514,22 +1598,17 @@ tmpaddr = -1; } - PRMSG (4,"SocketINETConnect: inet_addr(%s) = %x\n", - host, tmpaddr, 0); + PRMSG (4,"SocketINETConnect() inet_addr(%s) = %x\n", host, tmpaddr, 0); - if ((long)tmpaddr == -1L) - { - if ((hostp = _XGethostbyname(host,hparams)) == NULL) - { + if ((long)tmpaddr == -1L) { + if ((hostp = _XGethostbyname(host,hparams)) == NULL) { PRMSG (1,"SocketINETConnect: Can't get address for %s\n", host, 0, 0); ESET(EINVAL); return TRANS_CONNECT_FAILED; } - if (hostp->h_addrtype != AF_INET) /* is IP host? */ - { - PRMSG (1,"SocketINETConnect: not INET host%s\n", - host, 0, 0); + if (hostp->h_addrtype != AF_INET) { /* is IP host? */ + PRMSG (1,"SocketINETConnect: not INET host%s\n", host, 0, 0); ESET(EPROTOTYPE); return TRANS_CONNECT_FAILED; } @@ -1546,9 +1625,7 @@ sizeof (sockname.sin_addr)); #endif /* CRAY and OLDTCP */ - } -else - { + } else { #if defined(CRAY) && defined(OLDTCP) /* Only Cray UNICOS3 and UNICOS4 will define this */ sockname.sin_addr = tmpaddr; @@ -1563,18 +1640,14 @@ /* Check for number in the port string */ - if (!is_numeric (port)) - { - if ((servp = _XGetservbyname (port,"tcp",sparams)) == NULL) - { + if (!is_numeric (port)) { + if ((servp = _XGetservbyname (port,"tcp",sparams)) == NULL) { PRMSG (1,"SocketINETConnect: can't get service for %s\n", port, 0, 0); return TRANS_CONNECT_FAILED; } sockname.sin_port = htons (servp->s_port); - } - else - { + } else { tmpport = strtol (port, (char**)NULL, 10); if (tmpport < 1024 || tmpport > USHRT_MAX) return TRANS_CONNECT_FAILED; @@ -1585,7 +1658,7 @@ ntohs(sockname.sin_port), 0, 0); socketaddr = (struct sockaddr *) &sockname; socketaddrlen = sizeof(sockname); -#endif /* IPv6 */ + } /* * Turn on socket keepalive so the client process will eventually @@ -1636,10 +1709,14 @@ if (olderrno == ECONNREFUSED || olderrno == EINTR #if defined(IPv6) && defined(AF_INET6) - || ( ((addrlist->addr->ai_next != NULL) || + || (haveIPv6 && ((addrlist->addr->ai_next != NULL) || (addrlist->addr != addrlist->firstaddr)) && (olderrno == ENETUNREACH || olderrno == EAFNOSUPPORT || - olderrno == EADDRNOTAVAIL || olderrno == ETIMEDOUT)) + olderrno == EADDRNOTAVAIL || olderrno == ETIMEDOUT +#if defined(EHOSTDOWN) + || olderrno == EHOSTDOWN +#endif + )) #endif ) res = TRANS_TRY_CONNECT_AGAIN; @@ -1678,7 +1755,7 @@ } #if defined(IPv6) && defined(AF_INET6) - if (res != 0) { + if (haveIPv6 && res != 0) { addrlist->addr = addrlist->addr->ai_next; } #endif @@ -1702,15 +1779,20 @@ { char hostnamebuf[256]; +#if defined(IPv6) && defined(AF_INET6) + if (getaddrinfo == NULL) + haveIPv6 = 0; +#endif + TRANS(GetHostname) (hostnamebuf, sizeof (hostnamebuf)); if (strcmp (hostnamebuf, host) == 0) { return (1); } - else - { #if defined(IPv6) && defined(AF_INET6) + else if (haveIPv6) + { struct addrinfo *localhostaddr; struct addrinfo *otherhostaddr; struct addrinfo *i, *j; @@ -1756,7 +1838,10 @@ freeaddrinfo(localhostaddr); freeaddrinfo(otherhostaddr); return equiv; -#else + } +#endif + else + { /* * A host may have more than one network address. If any of the * network addresses of 'host' (specified to the connect call) @@ -1824,9 +1909,7 @@ i++; } - return (equiv); -#endif } }