Re: [PATCH net] netfilter: br_netfilter: reread nf_conn from skb after confirm()

在 2025/8/20 19:31, Florian Westphal 写道:

Wang Liang [off-list ref] wrote:

quoted

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.

Before the commit 2d72afb34065, __nf_conntrack_confirm() set
'ct->status |= IPS_CONFIRMED;' before check hash, the warning will not
happen, so I put it here.

As you say, the bug came in with 62e7151ae3eb. I will delete this paragraph
in next patch.

quoted

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.

Thanks for your suggestion!

Do you mean that ct may be deleted in confirm and return NF_DROP, so we can
not visit it in br_nf_local_in() and need to add 'case NF_DROP:' here?

I cannot find somewhere set skb->_nfct to NULL and return NF_DROP. Can you
give some hints?

------
Best regards
Wang Liang