| Message ID | 20170710130639.24167-1-fw@strlen.de |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Mon, Jul 10, 2017 at 03:06:39PM +0200, Florian Westphal wrote: > assuming we have lockless readers we should make sure they can only > see expectations that have already been initialized. > > hlist_add_head_rcu acts as memory barrier, move it after timer setup. > > Theoretically we could crash due to a del_timer() on other cpu > seeing garbage data. > > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > Not seen in practice, however i think this race > is real. Defer to nf-next if you think its not urgent. Applied to nf-next, thanks! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c index e03d16ed550d..c034951597fe 100644 --- a/net/netfilter/nf_conntrack_expect.c +++ b/net/netfilter/nf_conntrack_expect.c @@ -368,12 +368,6 @@ static void nf_ct_expect_insert(struct nf_conntrack_expect *exp) /* two references : one for hash insert, one for the timer */ refcount_add(2, &exp->use); - hlist_add_head_rcu(&exp->lnode, &master_help->expectations); - master_help->expecting[exp->class]++; - - hlist_add_head_rcu(&exp->hnode, &nf_ct_expect_hash[h]); - net->ct.expect_count++; - setup_timer(&exp->timeout, nf_ct_expectation_timed_out, (unsigned long)exp); helper = rcu_dereference_protected(master_help->helper, @@ -384,6 +378,12 @@ static void nf_ct_expect_insert(struct nf_conntrack_expect *exp) } add_timer(&exp->timeout); + hlist_add_head_rcu(&exp->lnode, &master_help->expectations); + master_help->expecting[exp->class]++; + + hlist_add_head_rcu(&exp->hnode, &nf_ct_expect_hash[h]); + net->ct.expect_count++; + NF_CT_STAT_INC(net, expect_create); }
assuming we have lockless readers we should make sure they can only see expectations that have already been initialized. hlist_add_head_rcu acts as memory barrier, move it after timer setup. Theoretically we could crash due to a del_timer() on other cpu seeing garbage data. Signed-off-by: Florian Westphal <fw@strlen.de> --- Not seen in practice, however i think this race is real. Defer to nf-next if you think its not urgent.