Message ID | 20200803182001.9243-1-ssuryaextr@gmail.com |
---|---|
State | Awaiting Upstream |
Delegated to: | David Miller |
Headers | show |
Series | [nf] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian | expand |
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.
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 = ®s->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
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;
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(-)