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

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

[Stephen Suryaputra]

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

Fixes: dbb5281a1f84 ("netfilter: nf_tables: add support for matching IPv4 options")
Fixes: c078ca3b0c5b ("netfilter: nft_exthdr: Add support for existence check")
Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
---
 net/netfilter/nft_exthdr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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);
 		return;
 	} else if (err < 0) {
 		goto err;
@@ -141,7 +141,7 @@ static void nft_exthdr_ipv4_eval(const struct nft_expr *expr,
 
 	err = ipv4_find_option(nft_net(pkt), skb, &offset, priv->type);
 	if (priv->flags & NFT_EXTHDR_F_PRESENT) {
-		*dest = (err >= 0);
+		*dest = cpu_to_le32(err >= 0);
 		return;
 	} else if (err < 0) {
 		goto err;
-- 
2.25.1

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

[Florian Westphal]

Stephen Suryaputra <ssuryaextr@gmail.com> 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.

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3757 bytes --]

Hi Stephen,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on nf/master]

url:    https://github.com/0day-ci/linux/commits/Stephen-Suryaputra/netfilter-nf_tables-nft_exthdr-the-presence-return-value-should-be-little-endian/20200804-055723
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
config: openrisc-randconfig-s032-20200804 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-117-g8c7aee71-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=openrisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> net/netfilter/nft_exthdr.c:47:23: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int [usertype] @@     got restricted __le32 [usertype] @@
>> net/netfilter/nft_exthdr.c:47:23: sparse:     expected unsigned int [usertype]
>> net/netfilter/nft_exthdr.c:47:23: sparse:     got restricted __le32 [usertype]
   net/netfilter/nft_exthdr.c:144:23: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int [usertype] @@     got restricted __le32 [usertype] @@
   net/netfilter/nft_exthdr.c:144:23: sparse:     expected unsigned int [usertype]
   net/netfilter/nft_exthdr.c:144:23: sparse:     got restricted __le32 [usertype]
   net/netfilter/nft_exthdr.c:264:33: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be16 [usertype] v16 @@     got unsigned short @@
   net/netfilter/nft_exthdr.c:264:33: sparse:     expected restricted __be16 [usertype] v16
   net/netfilter/nft_exthdr.c:264:33: sparse:     got unsigned short
   net/netfilter/nft_exthdr.c:284:33: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [assigned] [usertype] v32 @@     got unsigned int @@
   net/netfilter/nft_exthdr.c:284:33: sparse:     expected restricted __be32 [assigned] [usertype] v32
   net/netfilter/nft_exthdr.c:284:33: sparse:     got unsigned int
   net/netfilter/nft_exthdr.c:285:33: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be32 [usertype] v32 @@     got unsigned int @@
   net/netfilter/nft_exthdr.c:285:33: sparse:     expected restricted __be32 [usertype] v32
   net/netfilter/nft_exthdr.c:285:33: sparse:     got unsigned int

vim +47 net/netfilter/nft_exthdr.c

    35	
    36	static void nft_exthdr_ipv6_eval(const struct nft_expr *expr,
    37					 struct nft_regs *regs,
    38					 const struct nft_pktinfo *pkt)
    39	{
    40		struct nft_exthdr *priv = nft_expr_priv(expr);
    41		u32 *dest = &regs->data[priv->dreg];
    42		unsigned int offset = 0;
    43		int err;
    44	
    45		err = ipv6_find_hdr(pkt->skb, &offset, priv->type, NULL, NULL);
    46		if (priv->flags & NFT_EXTHDR_F_PRESENT) {
  > 47			*dest = cpu_to_le32(err >= 0);
    48			return;
    49		} else if (err < 0) {
    50			goto err;
    51		}
    52		offset += priv->offset;
    53	
    54		dest[priv->len / NFT_REG32_SIZE] = 0;
    55		if (skb_copy_bits(pkt->skb, offset, dest, priv->len) < 0)
    56			goto err;
    57		return;
    58	err:
    59		regs->verdict.code = NFT_BREAK;
    60	}
    61	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29749 bytes --]