Re: [PATCH nf] netfilter: conntrack: fix rmmod double-free race

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > @@ -934,8 +936,7 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct)
> >  
> >  	if (!nf_ct_ext_valid_post(ct->ext)) {
> >  		nf_ct_kill(ct);
> 
> I think this nf_ct_kill() delivers a destroy to userspace for an entry
> that was never added?
> 
> lock is not held anymore, so maybe a packet already got this
> conntrack.
> 
> Maybe post validation also needs to hold the lock and
> nf_conntrack_hash_check_insert() could undo the list insertion itself
> leaving things as they were before calling this function?

I think its safe to move _post before the actual insertion, because
get_next_corpse has to wait until we've released the lock.

If the genid check passes and the genid is incremented right
after on another core we can be sure that nf_ct_iterate() will see the
not-yet-inserted entry as we're holding the hash & reply slot locks.

This makes things simpler as we can go back to 'no failure after
insert' semantics.