RE: [PATCH 05/10] netfilter: nf_tables: fix mismatch in big-endian system

[David Laight <David.Laight@ACULAB.COM>]

From: Pablo Neira Ayuso
> Sent: 15 March 2017 17:01
> From: Liping Zhang <zlpnobody@gmail.com>
> 
> Currently, there are two different methods to store an u16 integer to
> the u32 data register. For example:
>   u32 *dest = &regs->data[priv->dreg];
>   1. *dest = 0; *(u16 *) dest = val_u16;
>   2. *dest = val_u16;
> 
> For method 1, the u16 value will be stored like this, either in
> big-endian or little-endian system:
>   0          15           31
>   +-+-+-+-+-+-+-+-+-+-+-+-+
>   |   Value   |     0     |
>   +-+-+-+-+-+-+-+-+-+-+-+-+
> 
> For method 2, in little-endian system, the u16 value will be the same
> as listed above. But in big-endian system, the u16 value will be stored
> like this:
>   0          15           31
>   +-+-+-+-+-+-+-+-+-+-+-+-+
>   |     0     |   Value   |
>   +-+-+-+-+-+-+-+-+-+-+-+-+
> 
> So later we use "memcmp(&regs->data[priv->sreg], data, 2);" to do
> compare in nft_cmp, nft_lookup expr ..., method 2 will get the wrong
> result in big-endian system, as 0~15 bits will always be zero.
> 
> For the similar reason, when loading an u16 value from the u32 data
> register, we should use "*(u16 *) sreg;" instead of "(u16)*sreg;",
> the 2nd method will get the wrong value in the big-endian system.
...

That seems to be papering over some of the obvious cracks.

The fact that the entire 32bits are zeroed makes me suspect that
in some places values are written as 8 bits and read later as 32.
The only way that can work on BE is if the values are always written
as 32 bit ones (assignment style 2).

OTOH using memcmp(,,2) relies on the data being in the lower addressed
bytes.

If the data does need to be in the lower addressed bytes I'd suggest
using a union rather than all the casts.

	David