[PATCH net 0/1] Netfilter fix for net

[PATCH net 0/1] Netfilter fix for net

[Pablo Neira Ayuso]

Hi,

The following batch contains one Netfilter fix for net:

1) conntrack offload bit is erroneously unset in a race scenario,
   from Florian Westphal.

Please, pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git nf-25-04-17

Thanks.

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

The following changes since commit 747fb8413aaa36e4c988d45c4fe20d4c2b0778cd:

  netlink: specs: ovs_vport: align with C codegen capabilities (2025-04-14 14:05:56 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git tags/nf-25-04-17

for you to fetch changes up to d2d31ea8cd80b9830cdab624e94f9d41178fc99d:

  netfilter: conntrack: fix erronous removal of offload bit (2025-04-17 11:14:22 +0200)

----------------------------------------------------------------
netfilter pull request 25-04-17

----------------------------------------------------------------
Florian Westphal (1):
      netfilter: conntrack: fix erronous removal of offload bit

 net/netfilter/nf_flow_table_core.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

[PATCH net 1/1] netfilter: conntrack: fix erronous removal of offload bit

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

The blamed commit exposes a possible issue with flow_offload_teardown():
We might remove the offload bit of a conntrack entry that has been
offloaded again.

1. conntrack entry c1 is offloaded via flow f1 (f1->ct == c1).
2. f1 times out and is pushed back to slowpath, c1 offload bit is
   removed.  Due to bug, f1 is not unlinked from rhashtable right away.
3. a new packet arrives for the flow and re-offload is triggered, i.e.
   f2->ct == c1.  This is because lookup in flowtable skip entries with
   teardown bit set.
4. Next flowtable gc cycle finds f1 again
5. flow_offload_teardown() is called again for f1 and c1 offload bit is
   removed again, even though we have f2 referencing the same entry.

This is harmless, but clearly not correct.
Fix the bug that exposes this: set 'teardown = true' to have the gc
callback unlink the flowtable entry from the table right away instead of
the unintentional defer to the next round.

Also prevent flow_offload_teardown() from fixing up the ct state more than
once: We could also be called from the data path or a notifier, not only
from the flowtable gc callback.

NF_FLOW_TEARDOWN can never be unset, so we can use it as synchronization
point: if we observe did not see a 0 -> 1 transition, then another CPU
is already doing the ct state fixups for us.

Fixes: 03428ca5cee9 ("netfilter: conntrack: rework offload nf_conn timeout extension logic")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_flow_table_core.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 9d8361526f82..9441ac3d8c1a 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -383,8 +383,8 @@ static void flow_offload_del(struct nf_flowtable *flow_table,
 void flow_offload_teardown(struct flow_offload *flow)
 {
 	clear_bit(IPS_OFFLOAD_BIT, &flow->ct->status);
-	set_bit(NF_FLOW_TEARDOWN, &flow->flags);
-	flow_offload_fixup_ct(flow);
+	if (!test_and_set_bit(NF_FLOW_TEARDOWN, &flow->flags))
+		flow_offload_fixup_ct(flow);
 }
 EXPORT_SYMBOL_GPL(flow_offload_teardown);
 
@@ -558,10 +558,12 @@ static void nf_flow_offload_gc_step(struct nf_flowtable *flow_table,
 
 	if (nf_flow_has_expired(flow) ||
 	    nf_ct_is_dying(flow->ct) ||
-	    nf_flow_custom_gc(flow_table, flow))
+	    nf_flow_custom_gc(flow_table, flow)) {
 		flow_offload_teardown(flow);
-	else if (!teardown)
+		teardown = true;
+	} else if (!teardown) {
 		nf_flow_table_extend_ct_timeout(flow->ct);
+	}
 
 	if (teardown) {
 		if (test_bit(NF_FLOW_HW, &flow->flags)) {
-- 
2.30.2

Re: [PATCH net 1/1] netfilter: conntrack: fix erronous removal of offload bit

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Pablo Neira Ayuso <pablo@netfilter.org>:

On Thu, 17 Apr 2025 12:28:47 +0200 you wrote:
> From: Florian Westphal <fw@strlen.de>
> 
> The blamed commit exposes a possible issue with flow_offload_teardown():
> We might remove the offload bit of a conntrack entry that has been
> offloaded again.
> 
> 1. conntrack entry c1 is offloaded via flow f1 (f1->ct == c1).
> 2. f1 times out and is pushed back to slowpath, c1 offload bit is
>    removed.  Due to bug, f1 is not unlinked from rhashtable right away.
> 3. a new packet arrives for the flow and re-offload is triggered, i.e.
>    f2->ct == c1.  This is because lookup in flowtable skip entries with
>    teardown bit set.
> 4. Next flowtable gc cycle finds f1 again
> 5. flow_offload_teardown() is called again for f1 and c1 offload bit is
>    removed again, even though we have f2 referencing the same entry.
> 
> [...]

Here is the summary with links:
  - [net,1/1] netfilter: conntrack: fix erronous removal of offload bit
    https://git.kernel.org/netdev/net/c/d2d31ea8cd80

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