netdev - wireless vs. alignment requirements

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Fri, 23 Nov 2007 21:33:21 +0100
From:	Johannes Berg <johannes@...solutions.net>
To:	Daniel Drake <dsd@...too.org>
Cc:	"David S. Miller" <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	linux-wireless <linux-wireless@...r.kernel.org>
Subject: wireless vs. alignment requirements

> David Miller found a problem in a wireless driver where I was using
> compare_ether_addr() on potentially unaligned data. Document that
> compare_ether_addr() is not safe for use everywhere, and add an equivalent
> function that works regardless of alignment.

FWIW, as I've said in the other thread I cannot believe this to be true
since the IP header is required to be aligned anyway. So *iff* this
really was a problem then we'd have much more problems and better fixed
the alignment of the received USB urb in zd1211rw.

Let me explain... This is going to take a bit of text...

Marking optional fields with < > for better readability, you have this
802.11 header layout (including byte sizes):

| FC (2) | durid (2) | A1 (6) | A2 (6) | A3 (6) | seq (2)
...	< optional A4 (6) > < optional qos (2) >
...	< optional HT (4) > < optional mesh (4 or 16) >
(NOTE: not sure about the order of the last two, this will probably
depend on the order in which 11n/11s are accepted! logically I would
expect the order as I wrote it)

(which makes it 24, 26, 30 or 32 bytes long plus the optional 11n/11s
additions which are multiples of four)

After that comes the data which may contain a SNAP header or such and
then the rest. The SNAP header is six bytes plus two byte ethertype, or
there could be a bridge tunnel header with six bytes plus two byte
ethertype, or it could be missing completely, in all of these cases it's
evenly divisible by four.

Now, the IP stack actually assumes that its header is four-byte aligned
(see comment at NET_IP_ALIGN, although it is not said explicitly that
the alignment requirement for an IP header is four) so that is actually
something for the hardware/firmware (!) to do, for example Broadcom
firmware will insert two bytes padding before the PLCP header (6 bytes,
sitting before the 802.11 header) if either the QoS field or A4 was
present (and the header length thus wasn't a multiple of four). Atheros
hardware will insert two bytes padding after the QoS field in the same
cases (IIRC).

Of course, you need to consider the RX header that sits before the
802.11 stuff. Broadcom's is 30 bytes, but if you add the six byte PLCP
header you have 36 bytes which is divisible by four. In the QoS XOR A4
case they add two bytes which makes it 38 with two bytes to spare for
4-byte alignment which are eaten up by the QoS/A4 field.

I don't know about any other hardware but considering that things work
as well as they do I'd think they do similar things.

Hence, going back to the 802.11 header and the IP header alignment
requirement, if we get the IP header alignment requirement right now I
cannot possibly see any way we would use compare_ether_addr() on an
address that is not at least two-byte aligned as required.

Whew. Or so I think.

johannes

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