[net,1/3] netfilter: nft_counter: Disable BH in nft_counter_offload_stats().

Commit Message

Pablo Neira Ayuso Aug. 22, 2024, 10:18 a.m. UTC

From: Sebastian Andrzej Siewior <[e-mail]>

The sequence counter nft_counter_seq is a per-CPU counter. There is no
lock associated with it. nft_counter_do_eval() is using the same counter
and disables BH which suggest that it can be invoked from a softirq.
This in turn means that nft_counter_offload_stats(), which disables only
preemption, can be interrupted by nft_counter_do_eval() leading to two
writer for one seqcount_t.
This can lead to loosing stats or reading statistics while they are
updated.

Disable BH during stats update in nft_counter_offload_stats() to ensure
one writer at a time.

Fixes: b72920f6e4a9d ("netfilter: nftables: counter hardware offload support")
Signed-off-by: Sebastian Andrzej Siewior <[e-mail]>
Reviewed-by: Florian Westphal <[e-mail]>
Signed-off-by: Pablo Neira Ayuso <[e-mail]>
---
 net/netfilter/nft_counter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

[e-mail] Aug. 22, 2024, 8:10 p.m. UTC | #1

Hello:

This series was applied to netdev/net.git (main)
by Pablo Neira Ayuso <[e-mail]>:

On Thu, 22 Aug 2024 12:18:40 +0200 you wrote:
> From: Sebastian Andrzej Siewior <[e-mail]>
> 
> The sequence counter nft_counter_seq is a per-CPU counter. There is no
> lock associated with it. nft_counter_do_eval() is using the same counter
> and disables BH which suggest that it can be invoked from a softirq.
> This in turn means that nft_counter_offload_stats(), which disables only
> preemption, can be interrupted by nft_counter_do_eval() leading to two
> writer for one seqcount_t.
> This can lead to loosing stats or reading statistics while they are
> updated.
> 
> [...]

Here is the summary with links:
  - [net,1/3] netfilter: nft_counter: Disable BH in nft_counter_offload_stats().
    https://git.kernel.org/netdev/net/c/1eacdd71b343
  - [net,2/3] netfilter: nft_counter: Synchronize nft_counter_reset() against reader.
    https://git.kernel.org/netdev/net/c/a0b39e2dc701
  - [net,3/3] netfilter: flowtable: validate vlan header
    https://git.kernel.org/netdev/net/c/6ea14ccb60c8

You are awesome, thank you!

Patch

diff --git a/net/netfilter/nft_counter.c b/net/netfilter/nft_counter.c
index 291ed2026367..16f40b503d37 100644
--- a/net/netfilter/nft_counter.c
+++ b/net/netfilter/nft_counter.c
@@ -265,7 +265,7 @@  static void nft_counter_offload_stats(struct nft_expr *expr,
 	struct nft_counter *this_cpu;
 	seqcount_t *myseq;
 
-	preempt_disable();
+	local_bh_disable();
 	this_cpu = this_cpu_ptr(priv->counter);
 	myseq = this_cpu_ptr(&nft_counter_seq);
 
@@ -273,7 +273,7 @@  static void nft_counter_offload_stats(struct nft_expr *expr,
 	this_cpu->packets += stats->pkts;
 	this_cpu->bytes += stats->bytes;
 	write_seqcount_end(myseq);
-	preempt_enable();
+	local_bh_enable();
 }
 
 void nft_counter_init_seqcount(void)