[PATCH] netfilter: ipset: fixes possible oops in mtype_resize

[PATCH] netfilter: ipset: fixes possible oops in mtype_resize

[Vasily Averin]

currently mtype_resize() can cause oops

        t = ip_set_alloc(htable_size(htable_bits));
        if (!t) {
                ret = -ENOMEM;
                goto out;
        }
        t->hregion = ip_set_alloc(ahash_sizeof_regions(htable_bits));

Increased htable_bits can force htable_size() to return 0.
In own turn ip_set_alloc(0) returns not 0 but ZERO_SIZE_PTR,
so follwoing access to t->hregion should trigger an OOPS.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 net/netfilter/ipset/ip_set_hash_gen.h | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 7d01086..7cd1d31 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -630,7 +630,7 @@ struct mtype_resize_ad {
 	struct htype *h = set->data;
 	struct htable *t, *orig;
 	u8 htable_bits;
-	size_t dsize = set->dsize;
+	size_t hsize, dsize = set->dsize;
 #ifdef IP_SET_HASH_WITH_NETS
 	u8 flags;
 	struct mtype_elem *tmp;
@@ -654,14 +654,12 @@ struct mtype_resize_ad {
 retry:
 	ret = 0;
 	htable_bits++;
-	if (!htable_bits) {
-		/* In case we have plenty of memory :-) */
-		pr_warn("Cannot increase the hashsize of set %s further\n",
-			set->name);
-		ret = -IPSET_ERR_HASH_FULL;
-		goto out;
-	}
-	t = ip_set_alloc(htable_size(htable_bits));
+	if (!htable_bits)
+		goto hbwarn;
+	hsize = htable_size(htable_bits);
+	if (!hsize)
+		goto hbwarn;
+	t = ip_set_alloc(hsize);
 	if (!t) {
 		ret = -ENOMEM;
 		goto out;
@@ -803,6 +801,12 @@ struct mtype_resize_ad {
 	if (ret == -EAGAIN)
 		goto retry;
 	goto out;
+
+hbwarn:
+	/* In case we have plenty of memory :-) */
+	pr_warn("Cannot increase the hashsize of set %s further\n", set->name);
+	ret = -IPSET_ERR_HASH_FULL;
+	goto out;
 }
 
 /* Get the current number of elements and ext_size in the set  */
-- 
1.8.3.1

Re: [PATCH] netfilter: ipset: fixes possible oops in mtype_resize

[Jozsef Kadlecsik]

Hi Vasily, Pablo,

On Thu, 17 Dec 2020, Vasily Averin wrote:

> currently mtype_resize() can cause oops
> 
>         t = ip_set_alloc(htable_size(htable_bits));
>         if (!t) {
>                 ret = -ENOMEM;
>                 goto out;
>         }
>         t->hregion = ip_set_alloc(ahash_sizeof_regions(htable_bits));
> 
> Increased htable_bits can force htable_size() to return 0.
> In own turn ip_set_alloc(0) returns not 0 but ZERO_SIZE_PTR,
> so follwoing access to t->hregion should trigger an OOPS.
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>

Good catch, thank you Vasily.

Acked-by: Jozsef Kadlecsik <kadlec@netfilter.org>

Best regards,
Jozsef

> ---
>  net/netfilter/ipset/ip_set_hash_gen.h | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
> index 7d01086..7cd1d31 100644
> --- a/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -630,7 +630,7 @@ struct mtype_resize_ad {
>  	struct htype *h = set->data;
>  	struct htable *t, *orig;
>  	u8 htable_bits;
> -	size_t dsize = set->dsize;
> +	size_t hsize, dsize = set->dsize;
>  #ifdef IP_SET_HASH_WITH_NETS
>  	u8 flags;
>  	struct mtype_elem *tmp;
> @@ -654,14 +654,12 @@ struct mtype_resize_ad {
>  retry:
>  	ret = 0;
>  	htable_bits++;
> -	if (!htable_bits) {
> -		/* In case we have plenty of memory :-) */
> -		pr_warn("Cannot increase the hashsize of set %s further\n",
> -			set->name);
> -		ret = -IPSET_ERR_HASH_FULL;
> -		goto out;
> -	}
> -	t = ip_set_alloc(htable_size(htable_bits));
> +	if (!htable_bits)
> +		goto hbwarn;
> +	hsize = htable_size(htable_bits);
> +	if (!hsize)
> +		goto hbwarn;
> +	t = ip_set_alloc(hsize);
>  	if (!t) {
>  		ret = -ENOMEM;
>  		goto out;
> @@ -803,6 +801,12 @@ struct mtype_resize_ad {
>  	if (ret == -EAGAIN)
>  		goto retry;
>  	goto out;
> +
> +hbwarn:
> +	/* In case we have plenty of memory :-) */
> +	pr_warn("Cannot increase the hashsize of set %s further\n", set->name);
> +	ret = -IPSET_ERR_HASH_FULL;
> +	goto out;
>  }
>  
>  /* Get the current number of elements and ext_size in the set  */
> -- 
> 1.8.3.1
> 
> 

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

Re: [PATCH] netfilter: ipset: fixes possible oops in mtype_resize

[Pablo Neira Ayuso]

On Thu, Dec 17, 2020 at 11:53:40AM +0300, Vasily Averin wrote:
> currently mtype_resize() can cause oops
> 
>         t = ip_set_alloc(htable_size(htable_bits));
>         if (!t) {
>                 ret = -ENOMEM;
>                 goto out;
>         }
>         t->hregion = ip_set_alloc(ahash_sizeof_regions(htable_bits));
> 
> Increased htable_bits can force htable_size() to return 0.
> In own turn ip_set_alloc(0) returns not 0 but ZERO_SIZE_PTR,
> so follwoing access to t->hregion should trigger an OOPS.

Applied, thanks.