ipset: Fix memory accounting for hash types on resize

If a fresh array block is allocated during resize, the current in-memory
set size should be increased by the size of the block, not replaced by it.

Before the fix, adding entries to a hash set type, leading to a table
resize, caused an inconsistent memory size to be reported. This becomes
more obvious when swapping sets with similar sizes:

  # cat hash_ip_size.sh
  #!/bin/sh
  FAIL_RETRIES=10

  tries=0
  while [ ${tries} -lt ${FAIL_RETRIES} ]; do
  	ipset create t1 hash:ip
  	for i in `seq 1 4345`; do
  		ipset add t1 1.2.$((i / 255)).$((i % 255))
  	done
  	t1_init="$(ipset list t1|sed -n 's/Size in memory: \(.*\)/\1/p')"

  	ipset create t2 hash:ip
  	for i in `seq 1 4360`; do
  		ipset add t2 1.2.$((i / 255)).$((i % 255))
  	done
  	t2_init="$(ipset list t2|sed -n 's/Size in memory: \(.*\)/\1/p')"

  	ipset swap t1 t2
  	t1_swap="$(ipset list t1|sed -n 's/Size in memory: \(.*\)/\1/p')"
  	t2_swap="$(ipset list t2|sed -n 's/Size in memory: \(.*\)/\1/p')"

  	ipset destroy t1
  	ipset destroy t2
  	tries=$((tries + 1))

  	if [ ${t1_init} -lt 10000 ] || [ ${t2_init} -lt 10000 ]; then
  		echo "FAIL after ${tries} tries:"
  		echo "T1 size ${t1_init}, after swap ${t1_swap}"
  		echo "T2 size ${t2_init}, after swap ${t2_swap}"
  		exit 1
  	fi
  done
  echo "PASS"
  # echo -n 'func hash_ip4_resize +p' > /sys/kernel/debug/dynamic_debug/control
  # ./hash_ip_size.sh
  [ 2035.018673] attempt to resize set t1 from 10 to 11, t 00000000fe6551fa
  [ 2035.078583] set t1 resized from 10 (00000000fe6551fa) to 11 (00000000172a0163)
  [ 2035.080353] Table destroy by resize 00000000fe6551fa
  FAIL after 4 tries:
  T1 size 9064, after swap 71128
  T2 size 71128, after swap 9064

Reported-by: NOYB <JunkYardMail1@Frontier.com>
Fixes: 9e41f26a505c ("netfilter: ipset: Count non-static extension memory for userspace")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 net/netfilter/ipset/ip_set_hash_gen.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Stefano,

On Sun, 26 May 2019, Stefano Brivio wrote:

> If a fresh array block is allocated during resize, the current in-memory
> set size should be increased by the size of the block, not replaced by it.
> 
> Before the fix, adding entries to a hash set type, leading to a table
> resize, caused an inconsistent memory size to be reported. This becomes
> more obvious when swapping sets with similar sizes:
> 
>   # cat hash_ip_size.sh
>   #!/bin/sh
>   FAIL_RETRIES=10
> 
>   tries=0
>   while [ ${tries} -lt ${FAIL_RETRIES} ]; do
>   	ipset create t1 hash:ip
>   	for i in `seq 1 4345`; do
>   		ipset add t1 1.2.$((i / 255)).$((i % 255))
>   	done
>   	t1_init="$(ipset list t1|sed -n 's/Size in memory: \(.*\)/\1/p')"
> 
>   	ipset create t2 hash:ip
>   	for i in `seq 1 4360`; do
>   		ipset add t2 1.2.$((i / 255)).$((i % 255))
>   	done
>   	t2_init="$(ipset list t2|sed -n 's/Size in memory: \(.*\)/\1/p')"
> 
>   	ipset swap t1 t2
>   	t1_swap="$(ipset list t1|sed -n 's/Size in memory: \(.*\)/\1/p')"
>   	t2_swap="$(ipset list t2|sed -n 's/Size in memory: \(.*\)/\1/p')"
> 
>   	ipset destroy t1
>   	ipset destroy t2
>   	tries=$((tries + 1))
> 
>   	if [ ${t1_init} -lt 10000 ] || [ ${t2_init} -lt 10000 ]; then
>   		echo "FAIL after ${tries} tries:"
>   		echo "T1 size ${t1_init}, after swap ${t1_swap}"
>   		echo "T2 size ${t2_init}, after swap ${t2_swap}"
>   		exit 1
>   	fi
>   done
>   echo "PASS"
>   # echo -n 'func hash_ip4_resize +p' > /sys/kernel/debug/dynamic_debug/control
>   # ./hash_ip_size.sh
>   [ 2035.018673] attempt to resize set t1 from 10 to 11, t 00000000fe6551fa
>   [ 2035.078583] set t1 resized from 10 (00000000fe6551fa) to 11 (00000000172a0163)
>   [ 2035.080353] Table destroy by resize 00000000fe6551fa
>   FAIL after 4 tries:
>   T1 size 9064, after swap 71128
>   T2 size 71128, after swap 9064
> 
> Reported-by: NOYB <JunkYardMail1@Frontier.com>
> Fixes: 9e41f26a505c ("netfilter: ipset: Count non-static extension memory for userspace")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Excellent, thanks! Patch is applied in ipset git tree and a new release is 
on the way.

Best regards,
Jozsef

> ---
>  net/netfilter/ipset/ip_set_hash_gen.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
> index 01d51f775f12..623e0d675725 100644
> --- a/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -625,7 +625,7 @@ mtype_resize(struct ip_set *set, bool retried)
>  					goto cleanup;
>  				}
>  				m->size = AHASH_INIT_SIZE;
> -				extsize = ext_size(AHASH_INIT_SIZE, dsize);
> +				extsize += ext_size(AHASH_INIT_SIZE, dsize);
>  				RCU_INIT_POINTER(hbucket(t, key), m);
>  			} else if (m->pos >= m->size) {
>  				struct hbucket *ht;
> -- 
> 2.20.1
> 
> 

-
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