[PATCH] netfilter: ipset: timeout values corrupted on set resize

[PATCH] netfilter: ipset: timeout values corrupted on set resize

[Josh Hunt]

If a resize is triggered on a set with timeouts enabled, the timeout
values will get corrupted when copying them to the new set. This occured
b/c the wrong timeout value is supplied to type_pf_elem_tadd().

This also adds simple debug statement similar to the one in type_pf_resize().

Signed-off-by: Josh Hunt <johunt@akamai.com>
---
 include/linux/netfilter/ipset/ip_set_ahash.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set_ahash.h b/include/linux/netfilter/ipset/ip_set_ahash.h
index ef9acd3..01d25e6 100644
--- a/include/linux/netfilter/ipset/ip_set_ahash.h
+++ b/include/linux/netfilter/ipset/ip_set_ahash.h
@@ -854,6 +854,8 @@ type_pf_tresize(struct ip_set *set, bool retried)
 retry:
 	ret = 0;
 	htable_bits++;
+	pr_debug("attempt to resize set %s from %u to %u, t %p\n",
+		 set->name, orig->htable_bits, htable_bits, orig);
 	if (!htable_bits) {
 		/* In case we have plenty of memory :-) */
 		pr_warning("Cannot increase the hashsize of set %s further\n",
@@ -873,7 +875,7 @@ retry:
 			data = ahash_tdata(n, j);
 			m = hbucket(t, HKEY(data, h->initval, htable_bits));
 			ret = type_pf_elem_tadd(m, data, AHASH_MAX(h), 0,
-						type_pf_data_timeout(data));
+						ip_set_timeout_get(type_pf_data_timeout(data)));
 			if (ret < 0) {
 				read_unlock_bh(&set->lock);
 				ahash_destroy(t);
-- 
1.7.0.4

Re: [PATCH] netfilter: ipset: timeout values corrupted on set resize

[Jozsef Kadlecsik]

Hi Josh,

On Tue, 19 Feb 2013, Josh Hunt wrote:

> If a resize is triggered on a set with timeouts enabled, the timeout
> values will get corrupted when copying them to the new set. This occured
> b/c the wrong timeout value is supplied to type_pf_elem_tadd().
> 
> This also adds simple debug statement similar to the one in type_pf_resize().

That's a good catch and perfect patch.

I'm just at the checking phase of the next release, which required a 
fairly large rewriting of the types - and which, as a side effect, fixed 
this bug. If I won't be able to release the rewritten version by the 
weekend, I'm going to apply your patch and release an interim version.

Best regards,
Jozsef
 
> Signed-off-by: Josh Hunt <johunt@akamai.com>
> ---
>  include/linux/netfilter/ipset/ip_set_ahash.h |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/netfilter/ipset/ip_set_ahash.h b/include/linux/netfilter/ipset/ip_set_ahash.h
> index ef9acd3..01d25e6 100644
> --- a/include/linux/netfilter/ipset/ip_set_ahash.h
> +++ b/include/linux/netfilter/ipset/ip_set_ahash.h
> @@ -854,6 +854,8 @@ type_pf_tresize(struct ip_set *set, bool retried)
>  retry:
>  	ret = 0;
>  	htable_bits++;
> +	pr_debug("attempt to resize set %s from %u to %u, t %p\n",
> +		 set->name, orig->htable_bits, htable_bits, orig);
>  	if (!htable_bits) {
>  		/* In case we have plenty of memory :-) */
>  		pr_warning("Cannot increase the hashsize of set %s further\n",
> @@ -873,7 +875,7 @@ retry:
>  			data = ahash_tdata(n, j);
>  			m = hbucket(t, HKEY(data, h->initval, htable_bits));
>  			ret = type_pf_elem_tadd(m, data, AHASH_MAX(h), 0,
> -						type_pf_data_timeout(data));
> +						ip_set_timeout_get(type_pf_data_timeout(data)));
>  			if (ret < 0) {
>  				read_unlock_bh(&set->lock);
>  				ahash_destroy(t);
> -- 
> 1.7.0.4
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] netfilter: ipset: timeout values corrupted on set resize

[Josh Hunt]

On 02/20/2013 03:44 PM, Jozsef Kadlecsik wrote:
>
> That's a good catch and perfect patch.
>
> I'm just at the checking phase of the next release, which required a
> fairly large rewriting of the types - and which, as a side effect, fixed
> this bug. If I won't be able to release the rewritten version by the
> weekend, I'm going to apply your patch and release an interim version.

Thanks Jozsef.

Is there a git tree I could check out to look at the new changes? I'd be 
interested in seeing what you've done. Also, what was the motivation 
behind the large changes you're doing?

Thanks
Josh

Re: [PATCH] netfilter: ipset: timeout values corrupted on set resize

[Jozsef Kadlecsik]

On Wed, 20 Feb 2013, Josh Hunt wrote:

> On 02/20/2013 03:44 PM, Jozsef Kadlecsik wrote:
> > 
> > That's a good catch and perfect patch.
> > 
> > I'm just at the checking phase of the next release, which required a
> > fairly large rewriting of the types - and which, as a side effect, fixed
> > this bug. If I won't be able to release the rewritten version by the
> > weekend, I'm going to apply your patch and release an interim version.
> 
> Is there a git tree I could check out to look at the new changes? I'd be
> interested in seeing what you've done.

It's on my devel laptop only :-)

> Also, what was the motivation behind the large changes you're doing?

The support of the extensions attached to the set members required a 
reorganization. Until now just the timeout as extension existed. The new 
extension to be introduced is the packet and bytes counters per elements. 
It was mandatory to rewrite the handling of the extensions and the 
generation of the variants of a set type.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] netfilter: ipset: timeout values corrupted on set resize

[Josh Hunt]

On 02/24/2013 01:52 PM, Jozsef Kadlecsik wrote:
>
> The support of the extensions attached to the set members required a
> reorganization. Until now just the timeout as extension existed. The new
> extension to be introduced is the packet and bytes counters per elements.
> It was mandatory to rewrite the handling of the extensions and the
> generation of the variants of a set type.
>

Very cool. I'll be very interested to try those out.

Thanks!
Josh

Re: [PATCH] netfilter: ipset: timeout values corrupted on set resize

[Mr Dash Four]

> The new 
> extension to be introduced is the packet and bytes counters per elements. 
>   
That would be superb - a feature truly missed since I started using 
ipsets. Would that count packet/bytes in sub-sets (members of set:list) 
as well?

Re: [PATCH] netfilter: ipset: timeout values corrupted on set resize

[Jozsef Kadlecsik]

On Mon, 25 Feb 2013, Mr Dash Four wrote:

> > The new extension to be introduced is the packet and bytes counters per
> > elements.   
> That would be superb - a feature truly missed since I started using ipsets.
> Would that count packet/bytes in sub-sets (members of set:list) as well?

Yes, the counters extension, similarly to the timeouts, will be supported 
in all set types.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] netfilter: ipset: timeout values corrupted on set resize

[Pablo Neira Ayuso]

Hi Jozsef,

On Wed, Feb 20, 2013 at 10:44:18PM +0100, Jozsef Kadlecsik wrote:
[...]
> That's a good catch and perfect patch.
> 
> I'm just at the checking phase of the next release, which required a 
> fairly large rewriting of the types - and which, as a side effect, fixed 
> this bug. If I won't be able to release the rewritten version by the 
> weekend, I'm going to apply your patch and release an interim version.

My suggestion is to apply the fix first so we can pass this to -stable.

Let me know. Thanks.