apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@attglobal.net>
Subject Re: [Patch] IPv6 on Tru64 broken
Date Tue, 02 Sep 2003 14:27:09 GMT
Colm MacCarthaigh wrote:

> On Mon, Aug 25, 2003 at 12:19:27PM -0400, Jeff Trawick wrote:
> 
>>If getaddrinfo() returns IPv6 addresses larger 
>>than that, we're hosed at that point and shouldn't be copying into the 
>>apr_sockaddr_info_t anyway.
> 
> 
> We get away with it, just about. Allthough the full length is
> copied; 
> 
>     429         new_sa->pool = p;
>     430         memcpy(&new_sa->sa, ai->ai_addr, ai->ai_addrlen);
>     431         apr_sockaddr_vars_set(new_sa, ai->ai_family, port);
> 
> it's salen itself that gets overwritten. This then gets reset back
> to a sensible value with the assignment in vars_set and the patch.
> It's moderately bold, but it works out fine.

we shouldn't be doing any overlaying of other fields...  if somebody 
moves the fields around it should still work...

(btw, prior to 1.0 the sockaddrs are to be moved to the end...  imagine 
if we copy 32 bytes there...  depending on the alignment, potentially we 
trash whatever was allocated next from the pool)

see additional patch I stuck at the bottom...  I don't know of a less 
clumsy way to get enough storage to avoid overlaying anything...

> Now, back to the reason why it does any of this; bind() is expecting
> a struct sockaddr_in6.sin6_addr to be passed, which would make the
> extra 4 bytes it's looking for correspond to sockaddr_in6.sin6_scope_id. 
> This makes sense in a freaky-dec-developer kind of way, though it really 
> is a bug. 
> 
> I'm guessing it means that the devlopers originally thought it might 
> be useful to allow people to bind() to scoped addresses , and to do 
> this you'd need an interface index (for the unitiated; IPv6 scoped 
> addresses are interface-specific).

In lieu of a complicated explanation, I'm just going to guess that the 
library and kernel are compiled with a different definition of 
sockaddr_in6 than user space.  This is the type of thing that is likely 
fixed before long.

> Actually now that I think about it, maybe the patch below is a
> better idea. 
> 
> 
>>(Alternatively, why am I confused?)
> 
> 
> I am too, but I've been playing with it all day now and it really
> does behave like this. Nothing on DU suprises me anymore.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Index: srclib/apr/network_io/unix/sockaddr.c
> ===================================================================
> RCS file: /home/cvspublic/apr/network_io/unix/sockaddr.c,v
> retrieving revision 1.42
> diff -u -u -r1.42 sockaddr.c
> --- srclib/apr/network_io/unix/sockaddr.c	15 Aug 2003 02:21:55 -0000	1.42
> +++ srclib/apr/network_io/unix/sockaddr.c	25 Aug 2003 16:55:21 -0000
> @@ -429,6 +429,19 @@
>          new_sa->pool = p;
>          memcpy(&new_sa->sa, ai->ai_addr, ai->ai_addrlen);
>          apr_sockaddr_vars_set(new_sa, ai->ai_family, port);
> +    
> +#if defined(OSF1) 
> +        /* Tru64 has a getaddrinfo/bind bug in that that it
> +         * returns and expects ai_addrlen = 32 for AF_INET6, 
> +         * even though sizeof(struct sockaddr_in6) == 28. 
> +         * Allthough the memcpy above has over-written salen, 
> +         * which then got reset by apr_sockaddr_vars we get away 
> +         * with it. Tru64 bind(), in it's infinite wisdom, 
> +         * doesn't bother checking the extra 4 bytes for global 
> +         * unicast IPv6 addresses. 
> +         */ 
> +        new_sa->salen = ai->ai_addrlen;
> +#endif
>  
>          if (!prev_sa) { /* first element in new list */
>              if (hostname) {

I would change the commentary to describe the real situation (nothing is 
getting overlaid if the patch below is included).

Can anyone suggest a better solution?

Index: include/apr_network_io.h
===================================================================
RCS file: /home/cvs/apr/include/apr_network_io.h,v
retrieving revision 1.141
diff -u -r1.141 apr_network_io.h
--- include/apr_network_io.h    30 May 2003 02:26:31 -0000      1.141
+++ include/apr_network_io.h    2 Sep 2003 14:13:23 -0000
@@ -252,6 +252,9 @@
          /** IPv6 sockaddr structure */
          struct sockaddr_in6 sin6;
  #endif
+#if defined(OSF1)
+        char reserve_enough_storage[32];
+#endif
      } sa;
      /** How big is the sockaddr we're using? */
      apr_socklen_t salen;

--/--

On the other hand, it is at least as reasonable to disable IPv6 support 
based on an autoconf test that catches the scenario where getaddrinfo() 
returns something bigger than sizeof(sockaddr_in6).  That helps keep the 
cruft down.

I think I'm leaning towards an autoconf test.  If somebody misses their 
IPv6 support and can determine that there is no OS fix available, we can 
make a patch available somewhere to work around the broken system.



Mime
View raw message