netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Netfilter fixes for net
@ 2020-08-15 10:31 Pablo Neira Ayuso
  2020-08-15 10:31 ` [PATCH 1/8] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian Pablo Neira Ayuso
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-15 10:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba

Hi,

The following patchset contains Netfilter fixes for net:

1) Endianness issue in IPv4 option support in nft_exthdr,
   from Stephen Suryaputra.

2) Removes the waitcount optimization in nft_compat,
   from Florian Westphal.

3) Remove ipv6 -> nf_defrag_ipv6 module dependency, from
   Florian Westphal.

4) Memleak in chain binding support, also from Florian.

5) Simplify nft_flowtable.sh selftest, from Fabian Frederick.

6) Optional MTU arguments for selftest nft_flowtable.sh,
   also from Fabian.

7) Remove noise error report when killing process in
   selftest nft_flowtable.sh, from Fabian Frederick.

8) Reject bogus getsockopt option length in ebtables,
   from Florian Westphal.

Please, pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Thank you.

----------------------------------------------------------------

The following changes since commit 7c7ab580db49cc7befe5f4b91bb1920cd6b07575:

  net: Convert to use the fallthrough macro (2020-08-08 14:29:09 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git HEAD

for you to fetch changes up to 5c04da55c754c44937b3d19c6522f9023fd5c5d5:

  netfilter: ebtables: reject bogus getopt len value (2020-08-14 11:59:08 +0200)

----------------------------------------------------------------
Fabian Frederick (3):
      selftests: netfilter: add checktool function
      selftests: netfilter: add MTU arguments to flowtables
      selftests: netfilter: kill running process only

Florian Westphal (4):
      netfilter: nft_compat: remove flush counter optimization
      netfilter: avoid ipv6 -> nf_defrag_ipv6 module dependency
      netfilter: nf_tables: free chain context when BINDING flag is missing
      netfilter: ebtables: reject bogus getopt len value

Stephen Suryaputra (1):
      netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian

 include/linux/netfilter_ipv6.h                     | 18 ------
 net/bridge/netfilter/ebtables.c                    |  4 ++
 net/bridge/netfilter/nf_conntrack_bridge.c         |  8 ++-
 net/ipv6/netfilter.c                               |  3 -
 net/netfilter/nf_tables_api.c                      |  6 +-
 net/netfilter/nft_compat.c                         | 37 +++++------
 net/netfilter/nft_exthdr.c                         |  4 +-
 tools/testing/selftests/netfilter/nft_flowtable.sh | 73 +++++++++++++---------
 8 files changed, 73 insertions(+), 80 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/8] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian
  2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
@ 2020-08-15 10:31 ` Pablo Neira Ayuso
  2020-08-15 10:31 ` [PATCH 2/8] netfilter: nft_compat: remove flush counter optimization Pablo Neira Ayuso
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-15 10:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba

From: Stephen Suryaputra <ssuryaextr@gmail.com>

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

v2: use nft_reg_store8() instead (Florian Westphal). Also to avoid the
    warnings reported by kernel test robot.

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>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 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..3c48cdc8935d 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);
+		nft_reg_store8(dest, 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);
+		nft_reg_store8(dest, err >= 0);
 		return;
 	} else if (err < 0) {
 		goto err;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/8] netfilter: nft_compat: remove flush counter optimization
  2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
  2020-08-15 10:31 ` [PATCH 1/8] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian Pablo Neira Ayuso
@ 2020-08-15 10:31 ` Pablo Neira Ayuso
  2020-08-15 10:31 ` [PATCH 3/8] netfilter: avoid ipv6 -> nf_defrag_ipv6 module dependency Pablo Neira Ayuso
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-15 10:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba

From: Florian Westphal <fw@strlen.de>

WARNING: CPU: 1 PID: 16059 at lib/refcount.c:31 refcount_warn_saturate+0xdf/0xf
[..]
 __nft_mt_tg_destroy+0x42/0x50 [nft_compat]
 nft_target_destroy+0x63/0x80 [nft_compat]
 nf_tables_expr_destroy+0x1b/0x30 [nf_tables]
 nf_tables_rule_destroy+0x3a/0x70 [nf_tables]
 nf_tables_exit_net+0x186/0x3d0 [nf_tables]

Happens when a compat expr is destoyed from abort path.
There is no functional impact; after this work queue is flushed
unconditionally if its pending.

This removes the waitcount optimization.  Test of repeated
iptables-restore of a ~60k kubernetes ruleset doesn't indicate
a slowdown.  In case the counter is needed after all for some workloads
we can revert this and increment the refcount for the
!= NFT_PREPARE_TRANS case to avoid the increment/decrement imbalance.

While at it, also flush for match case, this was an oversight
in the original patch.

Fixes: ffe8923f109b7e ("netfilter: nft_compat: make sure xtables destructors have run")
Reported-by: kernel test robot <rong.a.chen@intel.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_compat.c | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 6428856ccbec..8e56f353ff35 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -27,8 +27,6 @@ struct nft_xt_match_priv {
 	void *info;
 };
 
-static refcount_t nft_compat_pending_destroy = REFCOUNT_INIT(1);
-
 static int nft_compat_chain_validate_dependency(const struct nft_ctx *ctx,
 						const char *tablename)
 {
@@ -215,6 +213,17 @@ static int nft_parse_compat(const struct nlattr *attr, u16 *proto, bool *inv)
 	return 0;
 }
 
+static void nft_compat_wait_for_destructors(void)
+{
+	/* xtables matches or targets can have side effects, e.g.
+	 * creation/destruction of /proc files.
+	 * The xt ->destroy functions are run asynchronously from
+	 * work queue.  If we have pending invocations we thus
+	 * need to wait for those to finish.
+	 */
+	nf_tables_trans_destroy_flush_work();
+}
+
 static int
 nft_target_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
 		const struct nlattr * const tb[])
@@ -238,14 +247,7 @@ nft_target_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
 
 	nft_target_set_tgchk_param(&par, ctx, target, info, &e, proto, inv);
 
-	/* xtables matches or targets can have side effects, e.g.
-	 * creation/destruction of /proc files.
-	 * The xt ->destroy functions are run asynchronously from
-	 * work queue.  If we have pending invocations we thus
-	 * need to wait for those to finish.
-	 */
-	if (refcount_read(&nft_compat_pending_destroy) > 1)
-		nf_tables_trans_destroy_flush_work();
+	nft_compat_wait_for_destructors();
 
 	ret = xt_check_target(&par, size, proto, inv);
 	if (ret < 0)
@@ -260,7 +262,6 @@ nft_target_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
 
 static void __nft_mt_tg_destroy(struct module *me, const struct nft_expr *expr)
 {
-	refcount_dec(&nft_compat_pending_destroy);
 	module_put(me);
 	kfree(expr->ops);
 }
@@ -468,6 +469,8 @@ __nft_match_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
 
 	nft_match_set_mtchk_param(&par, ctx, match, info, &e, proto, inv);
 
+	nft_compat_wait_for_destructors();
+
 	return xt_check_match(&par, size, proto, inv);
 }
 
@@ -716,14 +719,6 @@ static const struct nfnetlink_subsystem nfnl_compat_subsys = {
 
 static struct nft_expr_type nft_match_type;
 
-static void nft_mt_tg_deactivate(const struct nft_ctx *ctx,
-				 const struct nft_expr *expr,
-				 enum nft_trans_phase phase)
-{
-	if (phase == NFT_TRANS_COMMIT)
-		refcount_inc(&nft_compat_pending_destroy);
-}
-
 static const struct nft_expr_ops *
 nft_match_select_ops(const struct nft_ctx *ctx,
 		     const struct nlattr * const tb[])
@@ -762,7 +757,6 @@ nft_match_select_ops(const struct nft_ctx *ctx,
 	ops->type = &nft_match_type;
 	ops->eval = nft_match_eval;
 	ops->init = nft_match_init;
-	ops->deactivate = nft_mt_tg_deactivate,
 	ops->destroy = nft_match_destroy;
 	ops->dump = nft_match_dump;
 	ops->validate = nft_match_validate;
@@ -853,7 +847,6 @@ nft_target_select_ops(const struct nft_ctx *ctx,
 	ops->size = NFT_EXPR_SIZE(XT_ALIGN(target->targetsize));
 	ops->init = nft_target_init;
 	ops->destroy = nft_target_destroy;
-	ops->deactivate = nft_mt_tg_deactivate,
 	ops->dump = nft_target_dump;
 	ops->validate = nft_target_validate;
 	ops->data = target;
@@ -917,8 +910,6 @@ static void __exit nft_compat_module_exit(void)
 	nfnetlink_subsys_unregister(&nfnl_compat_subsys);
 	nft_unregister_expr(&nft_target_type);
 	nft_unregister_expr(&nft_match_type);
-
-	WARN_ON_ONCE(refcount_read(&nft_compat_pending_destroy) != 1);
 }
 
 MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_NFT_COMPAT);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/8] netfilter: avoid ipv6 -> nf_defrag_ipv6 module dependency
  2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
  2020-08-15 10:31 ` [PATCH 1/8] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian Pablo Neira Ayuso
  2020-08-15 10:31 ` [PATCH 2/8] netfilter: nft_compat: remove flush counter optimization Pablo Neira Ayuso
@ 2020-08-15 10:31 ` Pablo Neira Ayuso
  2020-08-15 10:31 ` [PATCH 4/8] netfilter: nf_tables: free chain context when BINDING flag is missing Pablo Neira Ayuso
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-15 10:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba

From: Florian Westphal <fw@strlen.de>

nf_ct_frag6_gather is part of nf_defrag_ipv6.ko, not ipv6 core.

The current use of the netfilter ipv6 stub indirections  causes a module
dependency between ipv6 and nf_defrag_ipv6.

This prevents nf_defrag_ipv6 module from being removed because ipv6 can't
be unloaded.

Remove the indirection and always use a direct call.  This creates a
depency from nf_conntrack_bridge to nf_defrag_ipv6 instead:

modinfo nf_conntrack
depends:        nf_conntrack,nf_defrag_ipv6,bridge

.. and nf_conntrack already depends on nf_defrag_ipv6 anyway.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter_ipv6.h             | 18 ------------------
 net/bridge/netfilter/nf_conntrack_bridge.c |  8 ++++++--
 net/ipv6/netfilter.c                       |  3 ---
 3 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/include/linux/netfilter_ipv6.h b/include/linux/netfilter_ipv6.h
index aac42c28fe62..9b67394471e1 100644
--- a/include/linux/netfilter_ipv6.h
+++ b/include/linux/netfilter_ipv6.h
@@ -58,7 +58,6 @@ struct nf_ipv6_ops {
 			int (*output)(struct net *, struct sock *, struct sk_buff *));
 	int (*reroute)(struct sk_buff *skb, const struct nf_queue_entry *entry);
 #if IS_MODULE(CONFIG_IPV6)
-	int (*br_defrag)(struct net *net, struct sk_buff *skb, u32 user);
 	int (*br_fragment)(struct net *net, struct sock *sk,
 			   struct sk_buff *skb,
 			   struct nf_bridge_frag_data *data,
@@ -117,23 +116,6 @@ static inline int nf_ip6_route(struct net *net, struct dst_entry **dst,
 
 #include <net/netfilter/ipv6/nf_defrag_ipv6.h>
 
-static inline int nf_ipv6_br_defrag(struct net *net, struct sk_buff *skb,
-				    u32 user)
-{
-#if IS_MODULE(CONFIG_IPV6)
-	const struct nf_ipv6_ops *v6_ops = nf_get_ipv6_ops();
-
-	if (!v6_ops)
-		return 1;
-
-	return v6_ops->br_defrag(net, skb, user);
-#elif IS_BUILTIN(CONFIG_IPV6)
-	return nf_ct_frag6_gather(net, skb, user);
-#else
-	return 1;
-#endif
-}
-
 int br_ip6_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
 		    struct nf_bridge_frag_data *data,
 		    int (*output)(struct net *, struct sock *sk,
diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c b/net/bridge/netfilter/nf_conntrack_bridge.c
index 809673222382..8d033a75a766 100644
--- a/net/bridge/netfilter/nf_conntrack_bridge.c
+++ b/net/bridge/netfilter/nf_conntrack_bridge.c
@@ -168,6 +168,7 @@ static unsigned int nf_ct_br_defrag4(struct sk_buff *skb,
 static unsigned int nf_ct_br_defrag6(struct sk_buff *skb,
 				     const struct nf_hook_state *state)
 {
+#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
 	u16 zone_id = NF_CT_DEFAULT_ZONE_ID;
 	enum ip_conntrack_info ctinfo;
 	struct br_input_skb_cb cb;
@@ -180,14 +181,17 @@ static unsigned int nf_ct_br_defrag6(struct sk_buff *skb,
 
 	br_skb_cb_save(skb, &cb, sizeof(struct inet6_skb_parm));
 
-	err = nf_ipv6_br_defrag(state->net, skb,
-				IP_DEFRAG_CONNTRACK_BRIDGE_IN + zone_id);
+	err = nf_ct_frag6_gather(state->net, skb,
+				 IP_DEFRAG_CONNTRACK_BRIDGE_IN + zone_id);
 	/* queued */
 	if (err == -EINPROGRESS)
 		return NF_STOLEN;
 
 	br_skb_cb_restore(skb, &cb, IP6CB(skb)->frag_max_size);
 	return err == 0 ? NF_ACCEPT : NF_DROP;
+#else
+	return NF_ACCEPT;
+#endif
 }
 
 static int nf_ct_br_ip_check(const struct sk_buff *skb)
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index 409e79b84a83..6d0e942d082d 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -245,9 +245,6 @@ static const struct nf_ipv6_ops ipv6ops = {
 	.route_input		= ip6_route_input,
 	.fragment		= ip6_fragment,
 	.reroute		= nf_ip6_reroute,
-#if IS_MODULE(CONFIG_IPV6) && IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
-	.br_defrag		= nf_ct_frag6_gather,
-#endif
 #if IS_MODULE(CONFIG_IPV6)
 	.br_fragment		= br_ip6_fragment,
 #endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/8] netfilter: nf_tables: free chain context when BINDING flag is missing
  2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2020-08-15 10:31 ` [PATCH 3/8] netfilter: avoid ipv6 -> nf_defrag_ipv6 module dependency Pablo Neira Ayuso
@ 2020-08-15 10:31 ` Pablo Neira Ayuso
  2020-08-15 10:31 ` [PATCH 5/8] selftests: netfilter: add checktool function Pablo Neira Ayuso
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-15 10:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba

From: Florian Westphal <fw@strlen.de>

syzbot found a memory leak in nf_tables_addchain() because the chain
object is not free'd correctly on error.

Fixes: d0e2c7de92c7 ("netfilter: nf_tables: add NFT_CHAIN_BINDING")
Reported-by: syzbot+c99868fde67014f7e9f5@syzkaller.appspotmail.com
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index d878e34e3354..fd814e514f94 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2018,8 +2018,10 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
 	if (nla[NFTA_CHAIN_NAME]) {
 		chain->name = nla_strdup(nla[NFTA_CHAIN_NAME], GFP_KERNEL);
 	} else {
-		if (!(flags & NFT_CHAIN_BINDING))
-			return -EINVAL;
+		if (!(flags & NFT_CHAIN_BINDING)) {
+			err = -EINVAL;
+			goto err1;
+		}
 
 		snprintf(name, sizeof(name), "__chain%llu", ++chain_id);
 		chain->name = kstrdup(name, GFP_KERNEL);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/8] selftests: netfilter: add checktool function
  2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
                   ` (3 preceding siblings ...)
  2020-08-15 10:31 ` [PATCH 4/8] netfilter: nf_tables: free chain context when BINDING flag is missing Pablo Neira Ayuso
@ 2020-08-15 10:31 ` Pablo Neira Ayuso
  2020-08-15 10:31 ` [PATCH 6/8] selftests: netfilter: add MTU arguments to flowtables Pablo Neira Ayuso
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-15 10:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba

From: Fabian Frederick <fabf@skynet.be>

avoid repeating the same test for different toolcheck

Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 .../selftests/netfilter/nft_flowtable.sh      | 33 +++++++------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/netfilter/nft_flowtable.sh b/tools/testing/selftests/netfilter/nft_flowtable.sh
index d3e0809ab368..68a183753c6c 100755
--- a/tools/testing/selftests/netfilter/nft_flowtable.sh
+++ b/tools/testing/selftests/netfilter/nft_flowtable.sh
@@ -21,29 +21,18 @@ ns2out=""
 
 log_netns=$(sysctl -n net.netfilter.nf_log_all_netns)
 
-nft --version > /dev/null 2>&1
-if [ $? -ne 0 ];then
-	echo "SKIP: Could not run test without nft tool"
-	exit $ksft_skip
-fi
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
-	echo "SKIP: Could not run test without ip tool"
-	exit $ksft_skip
-fi
-
-which nc > /dev/null 2>&1
-if [ $? -ne 0 ];then
-	echo "SKIP: Could not run test without nc (netcat)"
-	exit $ksft_skip
-fi
+checktool (){
+	$1 > /dev/null 2>&1
+	if [ $? -ne 0 ];then
+		echo "SKIP: Could not $2"
+		exit $ksft_skip
+	fi
+}
 
-ip netns add nsr1
-if [ $? -ne 0 ];then
-	echo "SKIP: Could not create net namespace"
-	exit $ksft_skip
-fi
+checktool "nft --version" "run test without nft tool"
+checktool "ip -Version" "run test without ip tool"
+checktool "which nc" "run test without nc (netcat)"
+checktool "ip netns add nsr1" "create net namespace"
 
 ip netns add ns1
 ip netns add ns2
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/8] selftests: netfilter: add MTU arguments to flowtables
  2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
                   ` (4 preceding siblings ...)
  2020-08-15 10:31 ` [PATCH 5/8] selftests: netfilter: add checktool function Pablo Neira Ayuso
@ 2020-08-15 10:31 ` Pablo Neira Ayuso
  2020-08-15 10:32 ` [PATCH 7/8] selftests: netfilter: kill running process only Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-15 10:31 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba

From: Fabian Frederick <fabf@skynet.be>

Add some documentation, default values defined in original
script and Originator/Link/Responder arguments
using getopts like in tools/power/cpupower/bench/cpufreq-bench_plot.sh

Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 .../selftests/netfilter/nft_flowtable.sh      | 30 +++++++++++++++----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/netfilter/nft_flowtable.sh b/tools/testing/selftests/netfilter/nft_flowtable.sh
index 68a183753c6c..e98cac6f8bfd 100755
--- a/tools/testing/selftests/netfilter/nft_flowtable.sh
+++ b/tools/testing/selftests/netfilter/nft_flowtable.sh
@@ -2,13 +2,18 @@
 # SPDX-License-Identifier: GPL-2.0
 #
 # This tests basic flowtable functionality.
-# Creates following topology:
+# Creates following default topology:
 #
 # Originator (MTU 9000) <-Router1-> MTU 1500 <-Router2-> Responder (MTU 2000)
 # Router1 is the one doing flow offloading, Router2 has no special
 # purpose other than having a link that is smaller than either Originator
 # and responder, i.e. TCPMSS announced values are too large and will still
 # result in fragmentation and/or PMTU discovery.
+#
+# You can check with different Orgininator/Link/Responder MTU eg:
+# sh nft_flowtable.sh -o1000 -l500 -r100
+#
+
 
 # Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
@@ -78,11 +83,24 @@ ip -net nsr2 addr add dead:2::1/64 dev veth1
 # ns2 is going via nsr2 with a smaller mtu, so that TCPMSS announced by both peers
 # is NOT the lowest link mtu.
 
-ip -net nsr1 link set veth0 mtu 9000
-ip -net ns1 link set eth0 mtu 9000
+omtu=9000
+lmtu=1500
+rmtu=2000
+
+while getopts "o:l:r:" o
+do
+	case $o in
+		o) omtu=$OPTARG;;
+		l) lmtu=$OPTARG;;
+		r) rmtu=$OPTARG;;
+	esac
+done
+
+ip -net nsr1 link set veth0 mtu $omtu
+ip -net ns1 link set eth0 mtu $omtu
 
-ip -net nsr2 link set veth1 mtu 2000
-ip -net ns2 link set eth0 mtu 2000
+ip -net nsr2 link set veth1 mtu $rmtu
+ip -net ns2 link set eth0 mtu $rmtu
 
 # transfer-net between nsr1 and nsr2.
 # these addresses are not used for connections.
@@ -136,7 +154,7 @@ table inet filter {
       # as PMTUd is off.
       # This rule is deleted for the last test, when we expect PMTUd
       # to kick in and ensure all packets meet mtu requirements.
-      meta length gt 1500 accept comment something-to-grep-for
+      meta length gt $lmtu accept comment something-to-grep-for
 
       # next line blocks connection w.o. working offload.
       # we only do this for reverse dir, because we expect packets to
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/8] selftests: netfilter: kill running process only
  2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
                   ` (5 preceding siblings ...)
  2020-08-15 10:31 ` [PATCH 6/8] selftests: netfilter: add MTU arguments to flowtables Pablo Neira Ayuso
@ 2020-08-15 10:32 ` Pablo Neira Ayuso
  2020-08-15 10:32 ` [PATCH 8/8] netfilter: ebtables: reject bogus getopt len value Pablo Neira Ayuso
  2020-08-16 23:05 ` [PATCH 0/8] Netfilter fixes for net David Miller
  8 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-15 10:32 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba

From: Fabian Frederick <fabf@skynet.be>

Avoid noise like the following:
nft_flowtable.sh: line 250: kill: (4691) - No such process

Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 tools/testing/selftests/netfilter/nft_flowtable.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/netfilter/nft_flowtable.sh b/tools/testing/selftests/netfilter/nft_flowtable.sh
index e98cac6f8bfd..a47d1d832210 100755
--- a/tools/testing/selftests/netfilter/nft_flowtable.sh
+++ b/tools/testing/selftests/netfilter/nft_flowtable.sh
@@ -250,8 +250,14 @@ test_tcp_forwarding_ip()
 
 	sleep 3
 
-	kill $lpid
-	kill $cpid
+	if ps -p $lpid > /dev/null;then
+		kill $lpid
+	fi
+
+	if ps -p $cpid > /dev/null;then
+		kill $cpid
+	fi
+
 	wait
 
 	check_transfer "$ns1in" "$ns2out" "ns1 -> ns2"
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/8] netfilter: ebtables: reject bogus getopt len value
  2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
                   ` (6 preceding siblings ...)
  2020-08-15 10:32 ` [PATCH 7/8] selftests: netfilter: kill running process only Pablo Neira Ayuso
@ 2020-08-15 10:32 ` Pablo Neira Ayuso
  2020-08-16 23:05 ` [PATCH 0/8] Netfilter fixes for net David Miller
  8 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-15 10:32 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev, kuba

From: Florian Westphal <fw@strlen.de>

syzkaller reports splat:
------------[ cut here ]------------
Buffer overflow detected (80 < 137)!
Call Trace:
 do_ebt_get_ctl+0x2b4/0x790 net/bridge/netfilter/ebtables.c:2317
 nf_getsockopt+0x72/0xd0 net/netfilter/nf_sockopt.c:116
 ip_getsockopt net/ipv4/ip_sockglue.c:1778 [inline]

caused by a copy-to-user with a too-large "*len" value.
This adds a argument check on *len just like in the non-compat version
of the handler.

Before the "Fixes" commit, the reproducer fails with -EINVAL as
expected:
1. core calls the "compat" getsockopt version
2. compat getsockopt version detects the *len value is possibly
   in 64-bit layout (*len != compat_len)
3. compat getsockopt version delegates everything to native getsockopt
   version
4. native getsockopt rejects invalid *len

-> compat handler only sees len == sizeof(compat_struct) for GET_ENTRIES.

After the refactor, event sequence is:
1. getsockopt calls "compat" version (len != native_len)
2. compat version attempts to copy *len bytes, where *len is random
   value from userspace

Fixes: fc66de8e16ec ("netfilter/ebtables: clean up compat {get, set}sockopt handling")
Reported-by: syzbot+5accb5c62faa1d346480@syzkaller.appspotmail.com
Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/bridge/netfilter/ebtables.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 1641f414d1ba..ebe33b60efd6 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -2238,6 +2238,10 @@ static int compat_do_ebt_get_ctl(struct sock *sk, int cmd,
 	struct ebt_table *t;
 	struct net *net = sock_net(sk);
 
+	if ((cmd == EBT_SO_GET_INFO || cmd == EBT_SO_GET_INIT_INFO) &&
+	    *len != sizeof(struct compat_ebt_replace))
+		return -EINVAL;
+
 	if (copy_from_user(&tmp, user, sizeof(tmp)))
 		return -EFAULT;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/8] Netfilter fixes for net
  2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
                   ` (7 preceding siblings ...)
  2020-08-15 10:32 ` [PATCH 8/8] netfilter: ebtables: reject bogus getopt len value Pablo Neira Ayuso
@ 2020-08-16 23:05 ` David Miller
  8 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2020-08-16 23:05 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev, kuba

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Sat, 15 Aug 2020 12:31:53 +0200

> The following patchset contains Netfilter fixes for net:
> 
> 1) Endianness issue in IPv4 option support in nft_exthdr,
>    from Stephen Suryaputra.
> 
> 2) Removes the waitcount optimization in nft_compat,
>    from Florian Westphal.
> 
> 3) Remove ipv6 -> nf_defrag_ipv6 module dependency, from
>    Florian Westphal.
> 
> 4) Memleak in chain binding support, also from Florian.
> 
> 5) Simplify nft_flowtable.sh selftest, from Fabian Frederick.
> 
> 6) Optional MTU arguments for selftest nft_flowtable.sh,
>    also from Fabian.
> 
> 7) Remove noise error report when killing process in
>    selftest nft_flowtable.sh, from Fabian Frederick.
> 
> 8) Reject bogus getsockopt option length in ebtables,
>    from Florian Westphal.
> 
> Please, pull these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git

Pulled, thanks Pablo.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-08-16 23:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-15 10:31 [PATCH 0/8] Netfilter fixes for net Pablo Neira Ayuso
2020-08-15 10:31 ` [PATCH 1/8] netfilter: nf_tables: nft_exthdr: the presence return value should be little-endian Pablo Neira Ayuso
2020-08-15 10:31 ` [PATCH 2/8] netfilter: nft_compat: remove flush counter optimization Pablo Neira Ayuso
2020-08-15 10:31 ` [PATCH 3/8] netfilter: avoid ipv6 -> nf_defrag_ipv6 module dependency Pablo Neira Ayuso
2020-08-15 10:31 ` [PATCH 4/8] netfilter: nf_tables: free chain context when BINDING flag is missing Pablo Neira Ayuso
2020-08-15 10:31 ` [PATCH 5/8] selftests: netfilter: add checktool function Pablo Neira Ayuso
2020-08-15 10:31 ` [PATCH 6/8] selftests: netfilter: add MTU arguments to flowtables Pablo Neira Ayuso
2020-08-15 10:32 ` [PATCH 7/8] selftests: netfilter: kill running process only Pablo Neira Ayuso
2020-08-15 10:32 ` [PATCH 8/8] netfilter: ebtables: reject bogus getopt len value Pablo Neira Ayuso
2020-08-16 23:05 ` [PATCH 0/8] Netfilter fixes for net David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).