lkml.org 
[lkml]   [2014]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get
From
Date
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);





\
 
 \ /
  Last update: 2014-01-09 17:01    [W:0.222 / U:0.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site