* [PATCH nf] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian
@ 2020-08-03 18:20 Stephen Suryaputra
2020-08-03 22:15 ` Florian Westphal
2020-08-04 11:43 ` kernel test robot
0 siblings, 2 replies; 4+ messages in thread
From: Stephen Suryaputra @ 2020-08-03 18:20 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian
2020-08-03 18:20 [PATCH nf] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian Stephen Suryaputra
@ 2020-08-03 22:15 ` Florian Westphal
2020-08-04 11:43 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: Florian Westphal @ 2020-08-03 22:15 UTC (permalink / raw)
To: Stephen Suryaputra; +Cc: netfilter-devel, netdev
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian
2020-08-03 18:20 [PATCH nf] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian Stephen Suryaputra
@ 2020-08-04 11:43 ` kernel test robot
2020-08-04 11:43 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2020-08-04 11:43 UTC (permalink / raw)
To: Stephen Suryaputra, netfilter-devel
Cc: kbuild-all, netdev, Stephen Suryaputra
[-- 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 = ®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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29749 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian
@ 2020-08-04 11:43 ` kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2020-08-04 11:43 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3832 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 = ®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(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 29749 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-04 11:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-03 18:20 [PATCH nf] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian Stephen Suryaputra
2020-08-03 22:15 ` Florian Westphal
2020-08-04 11:43 ` kernel test robot
2020-08-04 11:43 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.