From 399a14ec7993d605740de7b2cd5c0ce8407d12ed Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Thu, 4 Aug 2022 19:26:27 +0200 Subject: netfilter: nf_tables: fix crash when nf_trace is enabled 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 Signed-off-by: Florian Westphal Signed-off-by: Jakub Kicinski --- 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 @@ next_rule: 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 @@ next_rule: 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); -- cgit From fe9e420defabd0f3b13239dd135d261fbeeb67ae Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Thu, 4 Aug 2022 19:26:28 +0200 Subject: selftests: netfilter: add test case for nf trace infrastructure 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 Signed-off-by: Jakub Kicinski --- .../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 -- cgit From b06ada6df9cf785099c142d96cb8a337ff46adf7 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Thu, 4 Aug 2022 19:26:29 +0200 Subject: netfilter: flowtable: fix incorrect Kconfig dependencies 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 Fixes: b038177636f8 ("netfilter: nf_flow_table: count pending offload workqueue tasks") Signed-off-by: Pablo Neira Ayuso Signed-off-by: Florian Westphal Signed-off-by: Jakub Kicinski --- 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. -- cgit