Re: [PATCH net 00/10] netfilter: updates for net

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Are sure that adding refcount bump is the way to go?

No, but I don't have a better idea at this time.

The obvious alternative is to grab a reference at expectation
creation time.

But this would pin the creating conntrack in memory.
In some cases, this will pin it forever, even if someone
runs 'conntrack -F/conntrack -D' to delete it if a helper
creates a forever-expectation.

> # git grep "nfct_help(exp->master)" net/netfilter/
> net/netfilter/nf_conntrack_expect.c:    struct nf_conn_help *master_help = nfct_help(exp->master);
> net/netfilter/nf_conntrack_expect.c:    struct nf_conn_help *master_help = nfct_help(exp->master);
> net/netfilter/nf_conntrack_helper.c:    struct nf_conn_help *help = nfct_help(exp->master);
> net/netfilter/nf_conntrack_netlink.c:   m_help = nfct_help(exp->master);
> net/netfilter/nf_conntrack_sip.c:                   nfct_help(exp->master)->helper != nfct_help(ct)->helper ||
> 
> These callsites need auditing.

Yes, but I don't have infinite time.  I am almost working non-stop
since these things got reported.
I cannot accelerate things any further.

> Yes, I just wonder if this can be fixed without adding checks
> everywhere in the code, I would need a bit more time too.

Another option is to stop releasing exp area with kfree()
and move back to kfree_rcu.  But still, I fear thats not enough.

What if we're releasing the master conntrack (refcount already 0)
and we're in object reuse scenario?

Then, master->ext can be krealloc'd in parallel.

TL;DR: I see no alternative to these refcount dances ATM and I also
think we need to add confirmed-bit check.

I hope I can condense this later with some new helper function that
can be used so we avoid open-coding this.

If you think that its better to yank these fixes and do the slow
audit, then fine, but I don't have any evidence that its a better
approach compared to incremental fixups.