netdev - Re: [PATCH] netfilter: ipset: ipset list may return wrong member count for set with timeout

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.11.1709112134060.12411@blackhole.kfki.hu>
Date:   Mon, 11 Sep 2017 21:36:38 +0200 (CEST)
From:   Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>
To:     Vishwanath Pai <vpai@...mai.com>
cc:     Pablo Neira Ayuso <pablo@...filter.org>, johunt@...mai.com,
        pai.vishwain@...il.com, netfilter-devel@...r.kernel.org,
        coreteam@...filter.org, netdev@...r.kernel.org
Subject: Re: [PATCH] netfilter: ipset: ipset list may return wrong member
 count for set with timeout

Hi,

Your patch is applied in the ipset git tree and I'm going to push it for 
kernel inclusion.

I modified the comment part: the elements counter can still be incorrect 
in the case of a huge set, because elements might time out during the 
listing.

Thanks for your patience!

Best regards,
Jozsef

On Thu, 17 Aug 2017, Vishwanath Pai wrote:

> Simple testcase:
> 
> $ ipset create test hash:ip timeout 5
> $ ipset add test 1.2.3.4
> $ ipset add test 1.2.2.2
> $ sleep 5
> 
> $ ipset l
> Name: test
> Type: hash:ip
> Revision: 5
> Header: family inet hashsize 1024 maxelem 65536 timeout 5
> Size in memory: 296
> References: 0
> Number of entries: 2
> Members:
> 
> We return "Number of entries: 2" but no members are listed. That is
> because mtype_list runs "ip_set_timeout_expired" and does not list the
> expired entries, but set->elements is never upated (until mtype_gc
> cleans it up later).
> 
> Reviewed-by: Joshua Hunt <johunt@...mai.com>
> Signed-off-by: Vishwanath Pai <vpai@...mai.com>
> ---
>  net/netfilter/ipset/ip_set_hash_gen.h | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
> index f236c0b..ff3d31c 100644
> --- a/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -1041,12 +1041,22 @@ struct htype {
>  static int
>  mtype_head(struct ip_set *set, struct sk_buff *skb)
>  {
> -	const struct htype *h = set->data;
> +	struct htype *h = set->data;
>  	const struct htable *t;
>  	struct nlattr *nested;
>  	size_t memsize;
>  	u8 htable_bits;
>  
> +	/* If any members have expired, set->elements will be wrong
> +	 * mytype_expire function will update it with the right count.
> +	 * we do not hold set->lock here, so grab it first.
> +	 */
> +	if (SET_WITH_TIMEOUT(set)) {
> +		spin_lock_bh(&set->lock);
> +		mtype_expire(set, h);
> +		spin_unlock_bh(&set->lock);
> +	}
> +
>  	rcu_read_lock_bh();
>  	t = rcu_dereference_bh_nfnl(h->table);
>  	memsize = mtype_ahash_memsize(h, t) + set->ext_size;
> -- 
> 1.9.1
> 
> 

-
E-mail  : kadlec@...ckhole.kfki.hu, kadlecsik.jozsef@...ner.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