Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> 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.
