Messages in this thread | | | Subject | Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get | From | Eric Dumazet <> | Date | Thu, 09 Jan 2014 07:23:45 -0800 |
| |
On Thu, 2014-01-09 at 09:24 +0400, Andrew Vagin wrote:
> I have one more question. Looks like I found another problem. > > init_conntrack: > hlist_nulls_add_head_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode, > &net->ct.unconfirmed); > > __nf_conntrack_hash_insert: > hlist_nulls_add_head_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode, > &net->ct.hash[hash]); > > We use one hlist_nulls_node to add a conntrack into two different lists. > As far as I understand, it's unacceptable in case of > SLAB_DESTROY_BY_RCU.
I guess you missed :
net/netfilter/nf_conntrack_core.c:1598: INIT_HLIST_NULLS_HEAD(&net->ct.unconfirmed, UNCONFIRMED_NULLS_VAL);
> > Lets imagine that we have two threads. The first one enumerates objects > from a first list, the second one recreates an object and insert it in a > second list. If a first thread in this moment stays on the object, it > can read "next", when it's in the second list, so it will continue > to enumerate objects from the second list. It is not what we want, isn't > it? > > cpu1 cpu2 > hlist_nulls_for_each_entry_rcu(ct) > destroy_conntrack > kmem_cache_free > > init_conntrack > hlist_nulls_add_head_rcu > ct = ct->next >
This will be fine.
I think we even have a counter to count number of occurrence of this rare event. (I personally never read a non null search_restart value)
NF_CT_STAT_INC(net, search_restart);
| |