Subject: Re: alignment crash in v6 ipfilter when receiving on gif
To: None <port-sparc64@NetBSD.org>
From: Pavel Cahyna <pavel@NetBSD.org>
List: port-sparc64
Date: 07/12/2007 10:57:33
On Wed, Jul 11, 2007 at 07:28:46PM -0400, der Mouse wrote:
> >>>> +			memcpy(&ip6_src, &ip6->ip6_src,
> >>>> +			       sizeof(struct in6_addr));
> >>> Unless we force a function call for memcpy, the compiler can still
> >>> assume that the source is 64bit aligned and ensure that the target
> >>> is 64bit aligned and to 64bit operations for the copy!
> >> No, it can't - there is no magic cast involved and it knows the
> >> alignment guarantees of ip6->ip6_src.
> > my reading of the above patch is that gcc could use 64 bit alignment
> > requiring load/stores because all the pointer types used are to 64
> > bit types.
> 
> My understanding - which of course may be wrong - is that that's
> somewhat of an oversimplification.
> 
> Rather, gcc knows that the source and destination are of types
> requiring 64-bit alignment, and that it may therefore assume the
> pointers are 64-bit aligned.
> 
> > pass a (real, not cast) "char *" or "void *" as the source, and GCC
> > won't think it's always going to be aligned.
> 
> I'm not sure.  It's possible to declare a char object having 64-bit
> alignment in gcc (__attribute__((__aligned__(8))) or some such); it's
> entirely possible that gcc will carry over alignment, so that
> (char *)&ip6_src is of type "pointer to char aligned to 64-bit
> boundary".  I'm not even sure it would be wrong to do so.
> 
> Declare ip6_src and/or ip6->ip6_src as __attribute__((__packed__)) and
> gcc should stop assuming that they have the alignment their type would
> normally call for, at least as I read extend.texi.
> 
> But the *right* fix is to stop overlaying a struct ip6_hdr onto a
> memory block not known to be correctly aligned for a struct ip6_hdr.
> Yes, coding that way is a bit of a pain.  But it's really the only way
> to be correct.  Anything less will come back to bite you in a sensitive
> spot at some future time, just the way it is here.

struct ip6_hdr is in fact declared with __attribute__((__packed__)).
Shouldn't it indicate that its members can be misaligned, too? Maybe the
problem is the cast (i6addr_t *)&ip6->ip6_src ?

Pavel