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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
