Re: [PATCH net] netfilter: br_netfilter: reread nf_conn from skb after confirm()
From: Florian Westphal <fw@strlen.de>
Date: 2025-08-20 11:31:49
Also in:
bridge, lkml, netfilter-devel
Subsystem:
ethernet bridge, netfilter, networking [general], the rest · Maintainers:
Nikolay Aleksandrov, Ido Schimmel, Pablo Neira Ayuso, Florian Westphal, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
Wang Liang [off-list ref] wrote:
Previous commit 2d72afb34065 ("netfilter: nf_conntrack: fix crash due to
removal of uninitialised entry") move the IPS_CONFIRMED assignment after
the hash table insertion.How is that related to this change? As you write below, the bug came in with 62e7151ae3eb.
quoted hunk ↗ jump to hunk
To solve the hash conflict, nf_ct_resolve_clash() try to merge the conntracks, and update skb->_nfct. However, br_nf_local_in() still use the old ct from local variable 'nfct' after confirm(), which leads to this issue. Fix it by rereading nfct from skb. Fixes: 62e7151ae3eb ("netfilter: bridge: confirm multicast packets before passing them up the stack") Signed-off-by: Wang Liang <redacted> --- net/bridge/br_netfilter_hooks.c | 1 + 1 file changed, 1 insertion(+)diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c index 94cbe967d1c1..55b1b7dcb609 100644 --- a/net/bridge/br_netfilter_hooks.c +++ b/net/bridge/br_netfilter_hooks.c@@ -626,6 +626,7 @@ static unsigned int br_nf_local_in(void *priv, break; } + nfct = skb_nfct(skb); ct = container_of(nfct, struct nf_conn, ct_general); WARN_ON_ONCE(!nf_ct_is_confirmed(ct));
There is a second bug here, confirm can return NF_DROP and nfct will be NULL. Can you make this change too? (or something similar)?
diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index 94cbe967d1c1..69b7b7c7565e 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c@@ -619,8 +619,9 @@ static unsigned int br_nf_local_in(void *priv, nf_bridge_pull_encap_header(skb); ret = ct_hook->confirm(skb); switch (ret & NF_VERDICT_MASK) { + case NF_DROP: case NF_STOLEN: - return NF_STOLEN; + return ret;
nfct reload seems correct, thanks for catching this.