netdev - Re: net: alignment problem in icmp code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20071022092350.5c75b3bd@poseidon.drzeus.cx>
Date:	Mon, 22 Oct 2007 09:23:50 +0200
From:	Pierre Ossman <drzeus@...eus.cx>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org
Subject: Re: net: alignment problem in icmp code

On Sun, 21 Oct 2007 22:15:24 -0700 (PDT)
David Miller <davem@...emloft.net> wrote:

> 
> Sure.  But the language defines that the types in question
> must be 64-bit aligned, so it is legal for the compiler to
> emit this code.
> 
> It's not a GCC bug.
> 

I've confirmed this behaviour on the AVR32 arch, and had a second look in the spec. The only way GCC can get away with this is by using a very creative interpretation of the result of casting a pointer to one type to a pointer to another type (which ISO C leaves undefined). Basically:

1. A cast from a pointer to one type to a pointer to a second type does not change the value. (GCC behaviour which we rely on heavily, even in your suggested patch).

2. A cast from a pointer to one type to a pointer to a second type changes the value so that it properly aligns with the second type. (As a cast to void*, which memcpy() requires, is safe according to ISO C, the pointer modification must be from char* to struct icmphdr*).

As 1 and 2 conflict, GCC's behaviour for pointer casts is still rather undefined. So although within the requirements of ISO C, it's rather user hostile. And it casts doubt on every pointer cast used.

> If you want to let the compiler know that a pointer to a type might
> not be aligned, you have to tell it so.

Which you can't do within the confines of ISO C. The get_unaligned() macro uses some GCC trickery (even outside what GCC documents as defined behaviour). To stay "clean", you have to avoid casting char* to foo*.

With all that said, as GCC does what it does, we can't really rely on a memcpy() anyway, so I support your patch.

As for other instances of unaligned accesses, is there any active work on getting rid of those? And would you accept more patches for fixing them? (Code complexity being the downside)

Rgds
Pierre

Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)