You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Colm MacCarthaigh <co...@stdlib.net> on 2003/08/25 17:52:38 UTC
[Patch] IPv6 on Tru64 broken
IPv6 with Apache on tru64 has been broken for a while now, and I finally
got round to figuring out what it was. It appears that Tru64's
getaddrinfo returns ai_addrlen of 32 for AF_INET6 address, but that
sizeof(struct sockaddr_in6) is 28.
This meant that apr_sockaddr_vars_set() was blindly setting salen
to 28, and then later when Apache called bind( , , 28) Tru64 bombed
out in unpleasant error land. Patch attached.
Since preserving the value for addrlen returned by getaddrinfo is
a good idea in any event I havn't made this #ifdef OSF1, there
may be more systems which rely on this behaviour and there's nothing
in the standards prohibiting it.
Also attached is a C prog to test which systems behave in this
manner;
bash-2.05a$ uname -a
OSF1 athene.heanet.ie V5.1 732 alpha
bash-2.05a$ gcc -o madness madness.c
bash-2.05a$ ./madness
sizeof(struct sockaddr_in6) = 28
addrinfo->ai_addrlen = 32
--
Colm MacCárthaigh Public Key: colm+pgp@stdlib.net
colm@stdlib.net http://www.stdlib.net/
Re: [Patch] IPv6 on Tru64 broken
Posted by Jeff Trawick <tr...@attglobal.net>.
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.
Re: [Patch] IPv6 on Tru64 broken
Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Mon, Aug 25, 2003 at 12:19:27PM -0400, Jeff Trawick wrote:
> >IPv6 with Apache on tru64 has been broken for a while now, and I finally
> >got round to figuring out what it was. It appears that Tru64's
> >getaddrinfo returns ai_addrlen of 32 for AF_INET6 address, but that
> >sizeof(struct sockaddr_in6) is 28.
> >
> >This meant that apr_sockaddr_vars_set() was blindly setting salen
> >to 28, and then later when Apache called bind( , , 28) Tru64 bombed
> >out in unpleasant error land. Patch attached.
>
> I'm not sure I would use the word "blindly", as we're using
> sizeof(sockaddr_in6).
True, but the return of getaddrinfo is being ignored is what I mean.
> 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.
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).
Now, because in Apache the address we pass is always global unicast
and IN6_IS_ADDR_LINKLOCAL/IN6_IS_ADDR_SITELOCAL/IN6_IS_ADDR_MULTICAST
are never going to evaluate to true, bind() never bothers looking
for the scope id. Which is a good thing since binding to the 32nd
system interface would probably not be the desired behaviour.
> Isn't the first step to see why getaddrinfo() is returning IPv6
> addresses larger than sizeof(sockaddr_in6) and take it from there?
I really should have included that. It's an OS bug, but it's
a consistent one. bind() simply fails if passed 28, and works
fine with 32, so we might aswell pass it what it wants. I
have it running right now at http://listserv.heanet.ie/ .
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.
--
Colm MacCárthaigh Public Key: colm+pgp@stdlib.net
colm@stdlib.net http://www.stdlib.net/
Re: [Patch] IPv6 on Tru64 broken
Posted by Jeff Trawick <tr...@attglobal.net>.
Colm MacCarthaigh wrote:
> IPv6 with Apache on tru64 has been broken for a while now, and I finally
> got round to figuring out what it was. It appears that Tru64's
> getaddrinfo returns ai_addrlen of 32 for AF_INET6 address, but that
> sizeof(struct sockaddr_in6) is 28.
>
> This meant that apr_sockaddr_vars_set() was blindly setting salen
> to 28, and then later when Apache called bind( , , 28) Tru64 bombed
> out in unpleasant error land. Patch attached.
I'm not sure I would use the word "blindly", as we're using
sizeof(sockaddr_in6). 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.
Isn't the first step to see why getaddrinfo() is returning IPv6
addresses larger than sizeof(sockaddr_in6) and take it from there?
(Alternatively, why am I confused?)