Re: [PATCH nf] netfilter: nft_connlimit: fix stale read of connection count

[Florian Westphal <fw@strlen.de>]

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> On 10/24/25 1:20 AM, Fernando Fernandez Mancera wrote:
> > nft_connlimit_eval() reads priv->list->count to check if the connection
> > limit has been exceeded. This value can be cached by the CPU while it
> > can be decremented by a different CPU when a connection is closed. This
> > causes a data race as the value cached might be outdated.
> > 
> > When a new connection is established and evaluated by the connlimit
> > expression, priv->list->count is incremented by nf_conncount_add(),
> > triggering the CPU's cache coherency protocol and therefore refreshing
> > the cached value before updating it.
> > 
> > Solve this situation by reading the value using READ_ONCE().
> > 
> > Fixes: df4a90250976 ("netfilter: nf_conncount: merge lookup and add functions")
> > Closes: https://lore.kernel.org/netfilter/trinity-85c72a88-d762-46c3-be97-36f10e5d9796-1761173693813@3c-app-mailcom-bs12/
> > Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
> > ---
> 
> While at this, I have found another problem with connlimit although with 
> this fix, it is partially mitigated. Since d265929930e2 ("netfilter: 
> nf_conncount: reduce unnecessary GC"), if __nf_conncount_add() is called 
> more than once during the same jiffy, the function won't check if the 
> connection is already tracked and will be added right away incrementing 
> the count. This can cause a situation where the count is greater than it 
> should and can cause nft_connlimit to match wrongly for a few jiffies.
> 
> I am open to suggestions on how to fix this.. as currently I don't have 
> a different one other than reverting the commit..

People are not supposed to use it in this way.

This is very expensive, there is a reason why iptables-extensions
examples all use iptables --syn.

This needs a documentation fix.

Or, we could revert df4a90250976 and then only _add for ctinfo NEW.