Re: [PATCH nf,v2] netfilter: conntrack: add dead flag to helpers

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, May 14, 2026 at 04:43:17PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > Add a new NF_CT_HELPER_F_DEAD helper flag to notify the packet path that
> > > this helper is going away. Thus, helpers are effectively disabled and no
> > > new expectations are created while removing the expectations created by
> > > this helper as well as unhelping the existing conntrack entries.
> > > 
> > > Add the check for NF_CT_HELPER_F_DEAD in the packet path to:
> > > - Conntrack confirmation path which invokes the helper callback.
> > > - Propagation of helper to conntrack via expectation.
> > > - OVS ct helper invocation.
> > 
> > Not sure this is enough.  New conntracks are not in any hash table /
> > unreachable, and synchronize_rcu() doesn't guarantee they get confirmed
> > (can get queued).
> 
> nf_ct_iterate_destroy() calls nf_queue_nf_hook_drop() for each netns.

But is that enough?  Consider:

cpu0						cpu1
						recieves verdict
						unlink from nfqueue list
drop_queued_packets (misses unlinked)
						... going on ..

I think to properly resolve this, there is a need to check
for this new dead flag after queueing to userspace (after its on list)
and again when receiving the verdict.

Arguably this is kind of different bug, because this comment is wrong:

/* a skb w. unconfirmed conntrack could have been reinjected just
 * before we called nf_queue_nf_hook_drop().
 *
 * This makes sure its inserted into conntrack table.
 */
 synchronize_net();

(it could have been requeued).

I think a more generic fix is to add a seqcnt to nf_queue_entry.
When queueing, record current seqcnt.

On reinject, drop if unconfirmed and seqcnt_now != entry->seqcnt.
Not nice, but I don't see a better way ATM.

The seqcnt can be pernet and it can be restricted to nfnetlink_queue.

Any better idea?