Subject: Re: crashes in ipfilter on i386
To: Greg Troxel <gdt@ir.bbn.com>
From: Darren Reed <darrenr@netbsd.org>
List: tech-net
Date: 09/13/2007 14:48:30
Greg Troxel wrote:
> Darren Reed <darrenr@netbsd.org> writes:
>
> > Greg Troxel wrote:
> >
> >> Here's the diff in my source tree, producing a kernel that doesn't
> >> crash.  Note there is a lot of s/INLINE// noise because I found it hard
> >> to follow the asm code, but the real change is dropping the IP6_NEQ
> >> test.  Obviously my change causes a functionality regression - it was
> >> intended to isolate the bug, not proposed as a fix.
> >>
> >> I still suspect failure to pull up enough bytes, but I can't point to
> >> the wrong line of code.
> >
> > I'd like you to try it without the #if 0 but with this change in ip_fil.h:
> >
> > ! #define       I60(x)  (((i6addr_t *)(x))->i6[0])
> > ! #define       I61(x)  (((i6addr_t *)(x))->i6[1])
> > ! #define       I62(x)  (((i6addr_t *)(x))->i6[2])
> > ! #define       I63(x)  (((i6addr_t *)(x))->i6[3])
> > ! #define       HI60(x) ntohl(((i6addr_t *)(x))->i6[0])
> > ! #define       HI61(x) ntohl(((i6addr_t *)(x))->i6[1])
> > ! #define       HI62(x) ntohl(((i6addr_t *)(x))->i6[2])
> > ! #define       HI63(x) ntohl(((i6addr_t *)(x))->i6[3])
> >
> >  #define       IP6_EQ(a,b)     ((I63(a) == I63(b)) && (I62(a) ==
> > I62(b)) && \
> >                         (I61(a) == I61(b)) && (I60(a) == I60(b)))
> > --- 156,169 ----
> >  #define       iplookupptr     vptr[0]
> >  #define       iplookupfunc    lptr[1]
> >
> > ! #define       I60(x)  (((u_32_t *)(x))[0])
> > ! #define       I61(x)  (((u_32_t *)(x))[1])
> > ! #define       I62(x)  (((u_32_t *)(x))[2])
> > ! #define       I63(x)  (((u_32_t *)(x))[3])
> > ! #define       HI60(x) ntohl(((u_32_t *)(x))[0])
> > ! #define       HI61(x) ntohl(((u_32_t *)(x))[1])
> > ! #define       HI62(x) ntohl(((u_32_t *)(x))[2])
> > ! #define       HI63(x) ntohl(((u_32_t *)(x))[3])
>
> I might not have mentioned: the system I'm having trouble on is i386 and
> netbsd-4.  The above fix was for strict alignment machines and the
> problem appeared on sparc64.
>
> I already have that change; it's been pulled up to netbsd-4 - it's
> precisely this commit:
>
> revision 1.6.12.5
> date: 2007/07/21 12:56:46;  author: liamjfoy;  state: Exp;  lines: +10 -9
> Pull up following revision(s) (requested by gdt in ticket #779):
> 	sys/dist/ipf/netinet/fil.c: revision 1.39
> 	sys/dist/ipf/netinet/ip_fil.h: revision 1.14
> Avoid casting to "i6addr_t *", because that type requires 64-bit
> alignment and nothing guarantees that IPv6 packets in mbufs are 8-byte
> aligned.  gcc was coalescing adjacent 32-bit compares into "ldx" on
> sparc64, leading to alignment faults when processing icmp6 arriving on
> gif with IPv4 outer addresses.
> Fix mostly from darrenr@.  Discussed extensively on port-sparc64.
>   

So you're saying that doesn't work?!
What assembly got generated as the result of the above?

Darren