Message ID | 49C93A8D.8000603@cosmosbay.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
Eric Dumazet wrote: > While working on this stuff, I found one suspect use of hlist_add_head() > > Its not a hot path, I believe following patch would make sure nothing > wrong happens. > > If a chain contains element A and B, then we might build a new table > with a new chain containing B and A (in this reverse order), and > a cpu could see A->next = B (new pointer), B->next = A (old pointer) I think you're right. > [PATCH] netfilter: Use hlist_add_head_rcu() in nf_conntrack_set_hashsize() > > Using hlist_add_head() in nf_conntrack_set_hashsize() is quite dangerous. > Without any barrier, one CPU could see a loop while doing its lookup. > Its true new table cannot be seen by another cpu, but previous table is still > readable. Applied, thanks Eric. -- To unsubscribe from this list: send the line "unsubscribe netdev" 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_core.c b/net/netfilter/nf_conntrack_core.c index 55befe5..54e983f 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -1121,7 +1121,7 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp) struct nf_conntrack_tuple_hash, hnode); hlist_del_rcu(&h->hnode); bucket = __hash_conntrack(&h->tuple, hashsize, rnd); - hlist_add_head(&h->hnode, &hash[bucket]); + hlist_add_head_rcu(&h->hnode, &hash[bucket]); } } old_size = nf_conntrack_htable_size;