On Fri, Feb 24, 2012 at 09:06:37AM +0100, Jozsef Kadlecsik wrote: > Hi Pablo, > > On Fri, 24 Feb 2012, Pablo Neira Ayuso wrote: > > > On Thu, Feb 23, 2012 at 09:44:21PM +0100, Jozsef Kadlecsik wrote: > > > OK, here it comes: > > > > > > The previous patch with the title "netfilter: fix soft lockup > > > when netlink adds new entries" introduced a race: conntrack and > > > ctnetlink could insert the same entry twice into the hash table. > > > The patch eliminates the race condition by using the same checking > > > as conntrack confirm does. > > > > > > Signed-off-by: Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx> > > > --- > > > include/net/netfilter/nf_conntrack.h | 2 + > > > net/netfilter/nf_conntrack_core.c | 41 ++++++++++++++++++++++++++++++++++ > > > net/netfilter/nf_conntrack_netlink.c | 9 +++---- > > > 3 files changed, 47 insertions(+), 5 deletions(-) > > > > > > diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h > > > index 8a2b0ae..48bfe75 100644 > > > --- a/include/net/netfilter/nf_conntrack.h > > > +++ b/include/net/netfilter/nf_conntrack.h > > > @@ -210,6 +210,8 @@ __nf_conntrack_find(struct net *net, u16 zone, > > > const struct nf_conntrack_tuple *tuple); > > > > > > extern void nf_conntrack_hash_insert(struct nf_conn *ct); > > > +extern int nf_conntrack_hash_check_insert(struct net *net, u16 zone, > > > + struct nf_conn *ct); > > > > nf_conntrack_hash_insert has no clients anymore after this change. > > > > We have two choices here: > > > > 1) Expand nf_conntrack_hash_insert to perform the checking inside. You can > > use the same prototype btw, net and zone can be extracted from the ct. > > It's a void function and we need a result: if the insert fails the > newly allocated entry must be destroyed and error reported to the user. > > > 2) For better code readability (and we save one exported symbol), add > > the code that you propose for nf_conntrack_hash_check_insert to > > nf_conntrack_netlink.c. I don't think we would ever have another client > > of nf_conntrack_hash_insert. > > That'd need exporting hash_conntrack and __nf_conntrack_hash_insert from > nf_conntrack_core.c. That was the reason why I instead introduced a new > function. I see. Go ahead then. Thanks Jozsef. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
