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

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> If the module reference grab does not work, maybe add:
>  
>         if (unlikely(nf_conntrack_ext_genid() != ext->id)
>                 return NULL;
>  
> to nfct_help() and nfct_timeout()? So access to these ct extension
> area is always validated before hand?
> 
> > > Another alternative would be to give up on this design completely
> > > and just grab module references :-)
> > 
> > But that would not be enough for userspace ct helpers, right?

This is a mess:

https://sashiko.dev/#/patchset/20260515103501.18669-1-fw%40strlen.de

No idea how to fix this yet.  Is it ok to disable cross-helper-attach
via ctnetlink?  I don't see a way to validate nfct_help_data().

Proposal: Get rid of data[] and nfct_help_data().  Explicit structure,
explicit helpers (e.g. nfct_help_data_sip(), type enum in nf_conn_help.

Callers must handle NULL return value everywhere (wrong helper type,
helper invalidated, etc).

unhelp will have to be changed to invoke the helper
destructor as well:

static int unhelp(struct nf_conn *ct, void *me)
{
        struct nf_conn_help *help = nfct_help(ct);

        if (help && rcu_dereference_raw(help->helper) == me) {
                nf_conntrack_event(IPCT_HELPER, ct);
                RCU_INIT_POINTER(help->helper, NULL);
        }

This can't be right, we lose the ->destroy() CB?

Ideally we could get rid of ->destroy, but that would require
permanent removal of pptp.