From owner-freebsd-pf@FreeBSD.ORG Thu Jun 16 12:39:49 2005 Return-Path: X-Original-To: freebsd-pf@freebsd.org Delivered-To: freebsd-pf@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E10BF16A41C; Thu, 16 Jun 2005 12:39:48 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailout2.pacific.net.au (mailout2.pacific.net.au [61.8.0.85]) by mx1.FreeBSD.org (Postfix) with ESMTP id 75DB743D5E; Thu, 16 Jun 2005 12:39:48 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy1.pacific.net.au (mailproxy1.pacific.net.au [61.8.0.86]) by mailout2.pacific.net.au (8.13.4/8.13.4/Debian-1) with ESMTP id j5GCdfYm026994; Thu, 16 Jun 2005 22:39:41 +1000 Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailproxy1.pacific.net.au (8.13.4/8.13.4/Debian-1) with ESMTP id j5GCdcae010499; Thu, 16 Jun 2005 22:39:39 +1000 Date: Thu, 16 Jun 2005 22:40:31 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Daniel Hartmeier In-Reply-To: <20050615223450.GY8526@insomnia.benzedrine.cx> Message-ID: <20050616220249.N833@delplex.bde.org> References: <200506132123.j5DLNove069255@freefall.freebsd.org> <20050615204232.GX8526@insomnia.benzedrine.cx> <20050615223450.GY8526@insomnia.benzedrine.cx> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-net@freebsd.org, Marcel Moolenaar , freebsd-pf@freebsd.org, Marcel Moolenaar Subject: Re: ia64/81284: Unaligned Reference with pf on 5.4/IA64 X-BeenThere: freebsd-pf@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Technical discussion and general questions about packet filter \(pf\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Jun 2005 12:39:49 -0000 On Thu, 16 Jun 2005, Daniel Hartmeier wrote: > 'packed', as I understand it, prohibits the compiler from inserting any > padding anywhere in the struct. That is, it guarantees that the total > size of a struct object equals the sum of the sizes of its members. It also requires the complier to generate specially pessimized code to access the struct. Otherwise even direct references like ip->ip_src might trap. > As a consequence, individual members can't be aligned properly if that > would require padding in front of them. The compiler can (and must?) > still align the first member, i.e. the beginning of the struct object, > though, no? No. This wouldn't be very useful, and wouldn't work right for things like arrays of packed structs -- the individual structs would have to be padded at the end since there can be no space between array elements, but packed structs are supposed to be unpadded at the end too. You can see that there is no alignment requirement for packed structs as a whole by looking at assembler output -- for a packed struct ip there is a .comm..,1 where the 1 says 1-byte alignment. > The IP header and the struct that represents it are defined very ^^^ were ... > carefully. It's no coincidence that ip_src/dst are placed where they > are: > > struct ip { > u_int ip_hl:4, /* header length */ > ip_v:4; /* version */ > u_char ip_tos; /* type of service */ > u_short ip_len; /* total length */ > u_short ip_id; /* identification */ > u_short ip_off; /* fragment offset field */ > u_char ip_ttl; /* time to live */ > u_char ip_p; /* protocol */ > u_short ip_sum; /* checksum */ > struct in_addr ip_src,ip_dst; /* source and dest address */ > } __packed; ^^^^^^^^^^ ... before this bug > > This guarantees that > > struct ip h; > &h.ip_src == (char *)&h + 12 > &h.ip_dst == (char *)&h + 16 > > i.e. they are both on 32-bit aligned if h itself is 32-bit aligned. > > If you look at any example in sys/netinet where struct ip members are > accessed, you see direct access to both u_short and uint32_t members. > Nowhere does anyone memcpy, bcopy or char-wise-copy ip_src/dst, for > instance. Except the compiler does this. > The issue also involves how the IP header is aligned within mbufs. > Functions usually get passed an mbuf pointer, and do > > struct mbuf *m; > struct ip *ip = mtod(m, struct ip *); > > and then happily access ip_src/dst without further alignment checks. mtod() just does a dubious cast. It depends on the data already being sufficiently aligned. For non-bogusly-packed IP headers this means 32-bit alignment (for the in_addr's). It's not clear where the misaligned headers come from. At least with gcc-3.3.3, I couldn't get a non-32-bit-aligned struct ip as a local variable by putting a char variable or char array before or after it. gcc has obscure rules for reordering local variables to pack them better, and this helps here. Padded structs withing (even unpadded) structs can easily be misaligned depending on what is before them, but struct ip's within structs seem to be rare. The one in ip_icmp.h is OK. Bruce