Subject: Re: [PATCH nf] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian

From: Florian Westphal <[email-masked]>
Date: Tue, 4 Aug 2020 00:15:34 +0200

Stephen Suryaputra <[email-masked]> wrote:
> On big-endian machine, the returned register data when the exthdr is
> present is not being compared correctly because little-endian is
> assumed. The function nft_cmp_fast_mask(), called by nft_cmp_fast_eval()
> and nft_cmp_fast_init(), calls cpu_to_le32().
> 
> The following dump also shows that little endian is assumed:
> 
> $ nft --debug=netlink add rule ip recordroute forward ip option rr exists counter
> ip
>   [ exthdr load ipv4 1b @ 7 + 0 present => reg 1 ]
>   [ cmp eq reg 1 0x01000000 ]
>   [ counter pkts 0 bytes 0 ]
> 
> Lastly, debug print in nft_cmp_fast_init() and nft_cmp_fast_eval() when
> RR option exists in the packet shows that the comparison fails because
> the assumption:
> 
> nft_cmp_fast_init:189 priv->sreg=4 desc.len=8 mask=0xff000000 data.data[0]=0x10003e0
> nft_cmp_fast_eval:57 regs->data[priv->sreg=4]=0x1 mask=0xff000000 priv->data=0x1000000

Right, nft userspace assumes a boolean data type when it does existence
check.

> diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
> index 07782836fad6..50e4935585e3 100644
> --- a/net/netfilter/nft_exthdr.c
> +++ b/net/netfilter/nft_exthdr.c
> @@ -44,7 +44,7 @@ static void nft_exthdr_ipv6_eval(const struct nft_expr *expr,
>  
>  	err = ipv6_find_hdr(pkt->skb, &offset, priv->type, NULL, NULL);
>  	if (priv->flags & NFT_EXTHDR_F_PRESENT) {
> -		*dest = (err >= 0);
> +		*dest = cpu_to_le32(err >= 0);

Both should probably use nft_reg_store8(dst, err >= 0) for consistency
with the rest.

But the patch looks correct to me, thanks.