Re: [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Jan 30, 2025 at 06:47:12PM +0100, Florian Westphal wrote:
> Given following input:
> table ip t {
>  chain c {
>   @ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0
>  }
> }
> 
> nft will produce following output:
> chain c {
>  @ih,48,16 set @ih,48,16 & 0x3f @ih,80,16 set @ih,80,16 & 0x3f0 @ih,160,32 set @ih,160,32 & 0x3fffff
> }
> 
> The input side is correct, the generated expressions sent to kernel are:
> 
> 1  [ payload load 2b @ inner header + 6 => reg 1 ]
> 2  [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x00000000 ]
> 3  [ payload write reg 1 => 2b @ inner header + 6 .. ]
> 4  [ payload load 2b @ inner header + 10 => reg 1 ]
> 5  [ bitwise reg 1 = ( reg 1 & 0x00000ffc ) ^ 0x00000000 ]
> 6  [ payload write reg 1 => 2b @ inner header + 10 .. ]
> 7  [ payload load 4b @ inner header + 20 => reg 1 ]
> 8  [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x00000000 ]
> 9  [ payload write reg 1 => 4b @ inner header + 20 .. ]
> 
> @ih,58,6 set 0 <- Zero 6 bits, starting with bit 58
> 
> Changes to inner header mandate a checksum update, which only works for
> even byte counts (except for last byte in the payload).
> 
> Thus, we load 2b at offet 6. (16bits, offset 48).
> 
> Because we want to zero 6 bits, we need a mask that retains 10 bits and
> clears 6: b1111111111000000 (first 8 bit retains 48-57, last 6 bit clear
> 58-63).  The '0xc0ff' is not correct, but thats because debug output comes
> from libnftnl which prints values in host byte order, the value will be
> interpreted as big endian on kernel side, so this will do the right thing.
> 
> Next, same problem:
> 
> @ih,86,6 set 0 <- Zero 6 bits, starting with bit 86.
> 
> nft needs to round down to even-sized byte offset, 10, then retain first
> 6 bits (80 + 6 == 86), then clear 6 bits (86-91), then keep 4 more as-is
> (92-95).
> 
> So mask is 0xfc0f (in big endian) would be correct (b1111110000001111).
> 
> Last expression, @ih,170,22 set 0, asks to clear 22 bits starting with bit
> 170, nft correctly rounds this down to a 32 bit read at offset 160.
> 
> Required mask keeps first 10 bits, then clears 22
> (b11111111110000000000000000000000).  Required mask would be 0xffc00000,
> which corresponds to the wrong-endian-printed value in line 8 above.
> 
> Now that we convinced ourselves that the input side is correct, fix up
> netlink delinearize to undo the mask alterations if we can't find a
> template to print a human-readable payload expression.
> 
> With this patch, we get this output:
> 
>   @ih,48,16 set @ih,48,16 & 0xffc0 @ih,80,16 set @ih,80,16 & 0xfc0f @ih,160,32 set @ih,160,32 & 0xffc00000
> 
> ... which isn't ideal.  We should fixup the payload expression to display
> the same output as the input, i.e. adjust payload->len and offset as per
> mask and discard the mask instead.
> 
> This will be done in a followup patch.
> 
> Fixes: 50ca788ca4d0 ("netlink: decode payload statment")
> Reported-by: Sunny73Cr <Sunny73Cr@protonmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>

Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>