Re: [PATCH v2 -next] netfilter: ctnetlink: attach expectations to unconfirmed conntracks

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Aug 07, 2013 at 06:27:19PM +0800, Gao feng wrote:
> > On 08/06/2013 09:19 PM, Pablo Neira Ayuso wrote:
> > > This patch adds the capability to attach expectations to unconfirmed
> > > conntrack entries. This patch is required by conntrack helpers that
> > > trigger expectations based on the first packet seen like the TFTP and
> > > the DHCPv6 user-space helpers.
> > > 
> > > There is no need to bump the conntrack refcount since unconfirmed
> > > conntracks are not yet in the hashes, thus, they are just referenced
> > > by one single packet. There is no trouble either in the get_next_corpse
> > > path, as unconfirmed conntracks only get their dying bit set to
> > > be discarded later on by nf_conntrack_confirm.
> > > 
> > 
> > The use after free problem still may happen.
> > 
> > the unconfirmed conntrack is created when we receive the first packet,
> > then this conntrack is linked in the per net global unconfirmed list.
> > 
> > And this conntrack may be destroyed when the return value of l4proto->packet
> > low than 0 in nf_conntrack_in.
> 
> I'm calling this from nfqueue, so the packet is retained until the
> user-space application issues the verdict on it, in that case that's
> not possible.
> 
> But that interface is generic and to get this working in all cases, I
> need the bump the refcount. So you're right, I'm going to revamp this.

Sorry, but I don't see how this approach will work in practice.

The kernel will frequently rip out entries from the unconfirmed list
and put them in the hash table, so on busy machines you'll often end up NOT
finding the conntrack you're looking for, because ct entry was moved to
the hash table and you hit the nulls element of some table bucket...