Message ID | alpine.LNX.2.01.1212161822470.27614@nerf07.vanv.qr |
---|---|
State | Not Applicable |
Headers | show |
On Sun, Dec 16, 2012 at 06:39:56PM +0100, Jan Engelhardt wrote: [...] > What would you think of the following patch? > > diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c > index 26a668a..0d17032 100644 > --- a/net/netfilter/xt_hashlimit.c > +++ b/net/netfilter/xt_hashlimit.c > @@ -591,9 +591,11 @@ hashlimit_mt(const struct sk_buff *skb, struct xt_action_param *par) > goto hotdrop; > > rcu_read_lock_bh(); > + spin_lock_bh(); > dh = dsthash_find(hinfo, &dst); > if (dh == NULL) { > dh = dsthash_alloc_init(hinfo, &dst); > + spin_unlock_bh(); This is sub-optimal and RCU becomes almost useless with this approach. The way to fix this is to follow a similar approach to what nf_conntrack does to avoid this race. > if (dh == NULL) { > rcu_read_unlock_bh(); > goto hotdrop; > @@ -601,6 +603,7 @@ hashlimit_mt(const struct sk_buff *skb, struct xt_action_param *par) > dh->expires = jiffies + msecs_to_jiffies(hinfo->cfg.expire); > rateinfo_init(dh, hinfo); > } else { > + spin_unlock_bh(); > /* update expiration timeout */ > dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire); > rateinfo_recalc(dh, now, hinfo->cfg.mode); -- 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
==== What would you think of the following patch? diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c index 26a668a..0d17032 100644 --- a/net/netfilter/xt_hashlimit.c +++ b/net/netfilter/xt_hashlimit.c @@ -591,9 +591,11 @@ hashlimit_mt(const struct sk_buff *skb, struct xt_action_param *par) goto hotdrop; rcu_read_lock_bh(); + spin_lock_bh(); dh = dsthash_find(hinfo, &dst); if (dh == NULL) { dh = dsthash_alloc_init(hinfo, &dst); + spin_unlock_bh(); if (dh == NULL) { rcu_read_unlock_bh(); goto hotdrop; @@ -601,6 +603,7 @@ hashlimit_mt(const struct sk_buff *skb, struct xt_action_param *par) dh->expires = jiffies + msecs_to_jiffies(hinfo->cfg.expire); rateinfo_init(dh, hinfo); } else { + spin_unlock_bh(); /* update expiration timeout */ dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire); rateinfo_recalc(dh, now, hinfo->cfg.mode);