[PATCH net 0/3] netfilter followup fixes for net

[PATCH net 0/3] netfilter followup fixes for net

[Florian Westphal]

Regressions, since 5.19:
Fix crash when packet tracing is enabled via 'meta nftrace set 1' rule.
Also comes with a test case.

Regressions, this cycle:
Fix Kconfig dependency for the flowtable /proc interface, we want this
to be off by default.

Florian Westphal (2):
  netfilter: nf_tables: fix crash when nf_trace is enabled
  selftests: netfilter: add test case for nf trace infrastructure

Pablo Neira Ayuso (1):
  netfilter: flowtable: fix incorrect Kconfig dependencies

 net/netfilter/Kconfig                         |  3 +-
 net/netfilter/nf_tables_core.c                | 21 +++--
 .../selftests/netfilter/nft_trans_stress.sh   | 81 +++++++++++++++++--
 3 files changed, 87 insertions(+), 18 deletions(-)

-- 
2.35.1

[PATCH net 1/3] netfilter: nf_tables: fix crash when nf_trace is enabled

[Florian Westphal]

do not access info->pkt when info->trace is not 1.
nft_traceinfo is not initialized, except when tracing is enabled.

The 'nft_trace_enabled' static key cannot be used for this, we must
always check info->trace first.

Pass nft_pktinfo directly to avoid this.

Fixes: e34b9ed96ce3 ("netfilter: nf_tables: avoid skb access on nf_stolen")
Reported-by: Hangbin Liu <liuhangbin@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_core.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index 3ddce24ac76d..cee3e4e905ec 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -34,25 +34,23 @@ static noinline void __nft_trace_packet(struct nft_traceinfo *info,
 	nft_trace_notify(info);
 }
 
-static inline void nft_trace_packet(struct nft_traceinfo *info,
+static inline void nft_trace_packet(const struct nft_pktinfo *pkt,
+				    struct nft_traceinfo *info,
 				    const struct nft_chain *chain,
 				    const struct nft_rule_dp *rule,
 				    enum nft_trace_types type)
 {
 	if (static_branch_unlikely(&nft_trace_enabled)) {
-		const struct nft_pktinfo *pkt = info->pkt;
-
 		info->nf_trace = pkt->skb->nf_trace;
 		info->rule = rule;
 		__nft_trace_packet(info, chain, type);
 	}
 }
 
-static inline void nft_trace_copy_nftrace(struct nft_traceinfo *info)
+static inline void nft_trace_copy_nftrace(const struct nft_pktinfo *pkt,
+					  struct nft_traceinfo *info)
 {
 	if (static_branch_unlikely(&nft_trace_enabled)) {
-		const struct nft_pktinfo *pkt = info->pkt;
-
 		if (info->trace)
 			info->nf_trace = pkt->skb->nf_trace;
 	}
@@ -96,7 +94,6 @@ static noinline void __nft_trace_verdict(struct nft_traceinfo *info,
 					 const struct nft_chain *chain,
 					 const struct nft_regs *regs)
 {
-	const struct nft_pktinfo *pkt = info->pkt;
 	enum nft_trace_types type;
 
 	switch (regs->verdict.code) {
@@ -110,7 +107,9 @@ static noinline void __nft_trace_verdict(struct nft_traceinfo *info,
 		break;
 	default:
 		type = NFT_TRACETYPE_RULE;
-		info->nf_trace = pkt->skb->nf_trace;
+
+		if (info->trace)
+			info->nf_trace = info->pkt->skb->nf_trace;
 		break;
 	}
 
@@ -271,10 +270,10 @@ nft_do_chain(struct nft_pktinfo *pkt, void *priv)
 		switch (regs.verdict.code) {
 		case NFT_BREAK:
 			regs.verdict.code = NFT_CONTINUE;
-			nft_trace_copy_nftrace(&info);
+			nft_trace_copy_nftrace(pkt, &info);
 			continue;
 		case NFT_CONTINUE:
-			nft_trace_packet(&info, chain, rule,
+			nft_trace_packet(pkt, &info, chain, rule,
 					 NFT_TRACETYPE_RULE);
 			continue;
 		}
@@ -318,7 +317,7 @@ nft_do_chain(struct nft_pktinfo *pkt, void *priv)
 		goto next_rule;
 	}
 
-	nft_trace_packet(&info, basechain, NULL, NFT_TRACETYPE_POLICY);
+	nft_trace_packet(pkt, &info, basechain, NULL, NFT_TRACETYPE_POLICY);
 
 	if (static_branch_unlikely(&nft_counters_enabled))
 		nft_update_chain_stats(basechain, pkt);
-- 
2.35.1

[PATCH net 2/3] selftests: netfilter: add test case for nf trace infrastructure

[Florian Westphal]

Enable/disable tracing infrastructure while packets are in-flight.
This triggers KASAN splat after
e34b9ed96ce3 ("netfilter: nf_tables: avoid skb access on nf_stolen").

While at it, reduce script run time as well.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../selftests/netfilter/nft_trans_stress.sh   | 81 +++++++++++++++++--
 1 file changed, 76 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/netfilter/nft_trans_stress.sh b/tools/testing/selftests/netfilter/nft_trans_stress.sh
index f1affd12c4b1..a7f62ad4f661 100755
--- a/tools/testing/selftests/netfilter/nft_trans_stress.sh
+++ b/tools/testing/selftests/netfilter/nft_trans_stress.sh
@@ -9,8 +9,27 @@
 # Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
 
-testns=testns1
+testns=testns-$(mktemp -u "XXXXXXXX")
+
 tables="foo bar baz quux"
+global_ret=0
+eret=0
+lret=0
+
+check_result()
+{
+	local r=$1
+	local OK="PASS"
+
+	if [ $r -ne 0 ] ;then
+		OK="FAIL"
+		global_ret=$r
+	fi
+
+	echo "$OK: nft $2 test returned $r"
+
+	eret=0
+}
 
 nft --version > /dev/null 2>&1
 if [ $? -ne 0 ];then
@@ -59,16 +78,66 @@ done)
 
 sleep 1
 
+ip netns exec "$testns" nft -f "$tmp"
 for i in $(seq 1 10) ; do ip netns exec "$testns" nft -f "$tmp" & done
 
 for table in $tables;do
-	randsleep=$((RANDOM%10))
+	randsleep=$((RANDOM%2))
 	sleep $randsleep
-	ip netns exec "$testns" nft delete table inet $table 2>/dev/null
+	ip netns exec "$testns" nft delete table inet $table
+	lret=$?
+	if [ $lret -ne 0 ]; then
+		eret=$lret
+	fi
 done
 
-randsleep=$((RANDOM%10))
-sleep $randsleep
+check_result $eret "add/delete"
+
+for i in $(seq 1 10) ; do
+	(echo "flush ruleset"; cat "$tmp") | ip netns exec "$testns" nft -f /dev/stdin
+
+	lret=$?
+	if [ $lret -ne 0 ]; then
+		eret=$lret
+	fi
+done
+
+check_result $eret "reload"
+
+for i in $(seq 1 10) ; do
+	(echo "flush ruleset"; cat "$tmp"
+	 echo "insert rule inet foo INPUT meta nftrace set 1"
+	 echo "insert rule inet foo OUTPUT meta nftrace set 1"
+	 ) | ip netns exec "$testns" nft -f /dev/stdin
+	lret=$?
+	if [ $lret -ne 0 ]; then
+		eret=$lret
+	fi
+
+	(echo "flush ruleset"; cat "$tmp"
+	 ) | ip netns exec "$testns" nft -f /dev/stdin
+
+	lret=$?
+	if [ $lret -ne 0 ]; then
+		eret=$lret
+	fi
+done
+
+check_result $eret "add/delete with nftrace enabled"
+
+echo "insert rule inet foo INPUT meta nftrace set 1" >> $tmp
+echo "insert rule inet foo OUTPUT meta nftrace set 1" >> $tmp
+
+for i in $(seq 1 10) ; do
+	(echo "flush ruleset"; cat "$tmp") | ip netns exec "$testns" nft -f /dev/stdin
+
+	lret=$?
+	if [ $lret -ne 0 ]; then
+		eret=1
+	fi
+done
+
+check_result $lret "add/delete with nftrace enabled"
 
 pkill -9 ping
 
@@ -76,3 +145,5 @@ wait
 
 rm -f "$tmp"
 ip netns del "$testns"
+
+exit $global_ret
-- 
2.35.1

[PATCH net 3/3] netfilter: flowtable: fix incorrect Kconfig dependencies

[Florian Westphal]

From: Pablo Neira Ayuso <pablo@netfilter.org>

Remove default to 'y', this infrastructure is not fundamental for the
flowtable operational.

Add a missing dependency on CONFIG_NF_FLOW_TABLE.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Fixes: b038177636f8 ("netfilter: nf_flow_table: count pending offload workqueue tasks")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index df6abbfe0079..22f15ebf6045 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -736,9 +736,8 @@ config NF_FLOW_TABLE
 
 config NF_FLOW_TABLE_PROCFS
 	bool "Supply flow table statistics in procfs"
-	default y
+	depends on NF_FLOW_TABLE
 	depends on PROC_FS
-	depends on SYSCTL
 	help
 	  This option enables for the flow table offload statistics
 	  to be shown in procfs under net/netfilter/nf_flowtable.
-- 
2.35.1

Re: [PATCH net 0/3] netfilter followup fixes for net

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  4 Aug 2022 19:26:26 +0200 you wrote:
> Regressions, since 5.19:
> Fix crash when packet tracing is enabled via 'meta nftrace set 1' rule.
> Also comes with a test case.
> 
> Regressions, this cycle:
> Fix Kconfig dependency for the flowtable /proc interface, we want this
> to be off by default.
> 
> [...]

Here is the summary with links:
  - [net,1/3] netfilter: nf_tables: fix crash when nf_trace is enabled
    https://git.kernel.org/netdev/net/c/399a14ec7993
  - [net,2/3] selftests: netfilter: add test case for nf trace infrastructure
    https://git.kernel.org/netdev/net/c/fe9e420defab
  - [net,3/3] netfilter: flowtable: fix incorrect Kconfig dependencies
    https://git.kernel.org/netdev/net/c/b06ada6df9cf

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html