Re: [PATCH] netfilter: xt_hashlimit: restore per-rule effectiveness

[Patrick McHardy <kaber@trash.net>]

Am 06.10.2010 08:28, schrieb Jan Engelhardt:
> When adding a second hashlimit rule with the same name, its parameters
> had no effect, because it had used a copy of the first one's inner
> state.

I'm not sure we can change this behaviour at this point. There's at
least one change in your patch that changes the default behaviour,
you can currently create a second rule for a table witout specifying
the mode and it will re-use the mode from the first rule. With your
patch this will result in an error due to the unfortunate fact that
the kernel doesn't check for valid modes. Having entries for rules
with different modes also would require to initalize all members
since a different rule might want to match on them. This again
doesn't work with the way masking is currently implemented.

> @@ -452,34 +456,34 @@ hashlimit_init_dst(const struct xt_hashlimit_htable *hinfo,
>  
>  	memset(dst, 0, sizeof(*dst));
>  
> -	switch (hinfo->family) {
> +	switch (family) {

This also looks problematic, the entries don't include the family
themselves, so you're allowing tables to contain entries of multiple
families, which might cause mismatches.

>  	case NFPROTO_IPV4:
> -		if (hinfo->cfg.mode & XT_HASHLIMIT_HASH_DIP)
> +		if (info->cfg.mode & XT_HASHLIMIT_HASH_DIP)
>  			dst->ip.dst = maskl(ip_hdr(skb)->daddr,
> -			              hinfo->cfg.dstmask);
> -		if (hinfo->cfg.mode & XT_HASHLIMIT_HASH_SIP)
> +			              info->cfg.dstmask);
> +		if (info->cfg.mode & XT_HASHLIMIT_HASH_SIP)
>  			dst->ip.src = maskl(ip_hdr(skb)->saddr,
> -			              hinfo->cfg.srcmask);
> +			              info->cfg.srcmask);
>  
> -		if (!(hinfo->cfg.mode &
> +		if (!(info->cfg.mode &
>  		      (XT_HASHLIMIT_HASH_DPT | XT_HASHLIMIT_HASH_SPT)))
>  			return 0;
>  		nexthdr = ip_hdr(skb)->protocol;
>  		break;