alignment faults in 3.6

On 9 October 2012 15:05, Scott Bambrough [off-list ref] wrote:

On 12-10-05 12:01 PM, Rob Herring wrote:

quoted

Here's a testcase. Compiled on ubuntu precise with
"arm-linux-gnueabihf-gcc -O2 -marm -march=armv7-a test.c".

typedef unsigned short u16;
typedef unsigned short __sum16;
typedef unsigned int __u32;
typedef unsigned char __u8;
typedef __u32 __be32;
typedef u16 __be16;

struct iphdr {
        __u8    ihl:4,
                version:4;
        __u8    tos;
        __be16  tot_len;
        __be16  id;
        __be16  frag_off;
        __u8    ttl;
        __u8    protocol;
        __sum16 check;
        __be32  saddr;
        __be32  daddr;
        /*The options start here. */
};

I was reading this thread with some interest.  AFAIK, with the default
alignment rules the above struct is packed; there will be no holes in it.

Correct.  The problem here is that something is passing around a pointer to
such a struct which is not 4-byte aligned as required by the ABI rules.

quoted

#define ntohl(x) __swab32((__u32)(__be32)(x))
#define IP_DF           0x4000          /* Flag: "Don't Fragment"       */

static inline __attribute__((const)) __u32 __swab32(__u32 x)
{
        __asm__ ("rev %0, %1" : "=r" (x) : "r" (x));
        return x;
}

int main(void * buffer, unsigned int *p_id)
{
        unsigned int id;
        int flush = 1;
        const struct iphdr *iph = buffer;
        __u32 len = *p_id;

        id = ntohl(*(__be32 *)&iph->id);

The above statement is the problem.  I think it is poorly written networking
code.  It takes the address of a 16 bit quantity (aligned on a halfword
address),

The 'id' member starts 4 bytes into the struct, so if the struct is properly
aligned, there will be no fault here.  The problem is that networking code does
not always align these structs correctly.

attempts to do a type conversion using pointers, then dereference
it.  I would have thought:

id = ntohs(iph->id);

would have been enough.

I'm assuming the is intentionally merging two 16-bit fields into one 32-bit
value.  What you suggest would do something rather different.

-- 
Mans Rullgard / mru