linux-kernel - Re: [PATCH] radix tree test suite: Fix a memory initialization issue

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] [day] [month] [year] [list]

Message-ID: <20230929195759.p7pgb6qtofdks2yy@revolver>
Date:   Fri, 29 Sep 2023 15:57:59 -0400
From:   "Liam R. Howlett" <Liam.Howlett@...cle.com>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     Matthew Wilcox <willy@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] radix tree test suite: Fix a memory initialization issue

* Christophe JAILLET <christophe.jaillet@...adoo.fr> [230928 17:11]:
> If __GFP_ZERO is used, the whole allocated memory should be cleared, not
> the first part of it only.
> 
> Fixes: cc86e0c2f306 ("radix tree test suite: add support for slab bulk APIs")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> ---
>  tools/testing/radix-tree/linux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/radix-tree/linux.c b/tools/testing/radix-tree/linux.c
> index d587a558997f..8ab162c48629 100644
> --- a/tools/testing/radix-tree/linux.c
> +++ b/tools/testing/radix-tree/linux.c
> @@ -172,7 +172,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *cachep, gfp_t gfp, size_t size,
>  			if (cachep->ctor)
>  				cachep->ctor(p[i]);
>  			else if (gfp & __GFP_ZERO)
> -				memset(p[i], 0, cachep->size);
> +				memset(p[i], 0, cachep->size * size);
>  		}
>  	}

This doesn't look right.  In fact, I think you have taken a bug and
extended it to clear the bugs over-allocation worth of memory.

The bulk allocator allocates to the array (here it's p) of a given size
(size).  Each kmem_cache has a size pre-set, that is, each allocation
from that cache is of size cachep->size.

What you are doing is zeroing each pointer for the entire size of the
allocation for the entire range, and not just one elements worth. This
isn't crashing because we are allocating too much memory to begin with.

Note that this zeroing is in a for loop:
	for (i = 0; i < size; i++) {
		...
	}

So, really the bug is that we are over-allocating and not that we aren't
zeroing the entire thing.

If you drop size from these arguments and use LSAN to check that things
are not overrun, then you will see that we can reduce the allocation by
a lot and still run correctly.

Thanks,
Liam