linux-kernel - Re: [PATCH 2/2] maple_tree: Add some alloc node test case

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: <6zslkyydiamukudqgakplkxfr2ie6jfdnltfinnishseb3twbk@eszq5q3gkgwo>
Date: Tue, 15 Oct 2024 09:52:58 -0400
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Wei Yang <richard.weiyang@...il.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
        Jiazi Li <jqqlijiazi@...il.com>, linux-kernel@...r.kernel.org,
        maple-tree@...ts.infradead.org
Subject: Re: [PATCH 2/2] maple_tree: Add some alloc node test case

* Wei Yang <richard.weiyang@...il.com> [241015 09:31]:
> On Mon, Oct 14, 2024 at 09:15:07PM -0400, Liam R. Howlett wrote:
> >* Wei Yang <richard.weiyang@...il.com> [241010 21:18]:
> >> On Wed, Jun 26, 2024 at 12:06:31PM -0400, Liam R. Howlett wrote:
> >> >From: Jiazi Li <jqqlijiazi@...il.com>
> >> >
> >> >Add some maple_tree alloc node tese case.
> >> >
> >> >Suggested-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> >> >Signed-off-by: Jiazi Li <jqqlijiazi@...il.com>
> >> >Signed-off-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> >> >---
> >> > tools/testing/radix-tree/maple.c | 22 ++++++++++++++++++++++
> >> > 1 file changed, 22 insertions(+)
> >> >
> >> >diff --git a/tools/testing/radix-tree/maple.c b/tools/testing/radix-tree/maple.c
> >> >index 11f1efdf83f9..b4b5fd9f294d 100644
> >> >--- a/tools/testing/radix-tree/maple.c
> >> >+++ b/tools/testing/radix-tree/maple.c
> >> >@@ -462,6 +462,28 @@ static noinline void __init check_new_node(struct maple_tree *mt)
> >> > 	MT_BUG_ON(mt, mas_allocated(&mas) != 10 + MAPLE_ALLOC_SLOTS - 1);
> >> > 	mas_destroy(&mas);
> >> > 
> >> >+	mas.node = MA_ERROR(-ENOMEM);
> >> >+	mas_node_count(&mas, MAPLE_ALLOC_SLOTS + 1); /* Request */
> >> >+	mas_nomem(&mas, GFP_KERNEL); /* Fill request */
> >> 
> >> I am not sure why mas_nomem() is here.
> >> 
> >> Without this one, we still can trigger the original bug.
> >
> >It will fill the maple state allocation.  Might not be needed but
> >doesn't hurt.
> >
> 
> I took another look at it. We really need mas_nomem() here, since we call
> mt_set_non_kernel(0) at the beginning of check_new_node(). 
> 
> So mas_node_count() just set the request count and mas_nomem() does the real
> allocation.
> 
> Any reason to design test case like this?

There are two types of testing that we do: ones that can call internal
functions to poke at stuff (like this one), and ones that can't which is
more like what users can do (lives in lib/test_maple_tree.c).

Both are important as this checks what we think will happen while the
other tests the integration of all our work together.

As for this particular test, the two parts work together most of the
time, but if we run out of memory then we need to test them in parts to
make sure both are doing what we think.

It also means that we can easily set up tests for scenarios that are
harder to synthesize using the external api - but they can happen.  I'm
not sure if this is the case for this test, but there were tests already
setting the allocations into certain positions and this is now just
another one of those tests.

I hope this helps explain some more of the testing work here as it is
extremely important.  I'm very pleased to have this setup to recreate
issues for both fixing issues and ensuring they do not return -
especially on large refactoring, new features or algorithms.

Thanks,
Liam