linux-kernel - Re: [PATCH v8 14/21] mm/mmap: Avoid zeroing vma tree in mmap

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: <311acc28-50fd-4acd-b5e5-fa03ae4ab2cd@lucifer.local>
Date: Fri, 4 Oct 2024 15:58:00 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Bert Karwatzki <spasswolf@....de>
Cc: "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 14/21] mm/mmap: Avoid zeroing vma tree in mmap_region()

On Fri, Oct 04, 2024 at 03:32:28PM +0100, Lorenzo Stoakes wrote:
> On Fri, Oct 04, 2024 at 03:26:26PM +0100, Lorenzo Stoakes wrote:
> > On Fri, Oct 04, 2024 at 03:23:37PM +0100, Lorenzo Stoakes wrote:
> > > On Fri, Oct 04, 2024 at 11:35:44AM +0200, Bert Karwatzki wrote:
> > > > Here's the log procduced by this kernel:
> > > >
> > > > c9e7f76815d3 (HEAD -> maple_tree_debug_4) hack: set of info stuff v5
> > > > 7e3bb072761a mm: correct error handling in mmap_region()
> > > > 77df9e4bb222 (tag: next-20241001, origin/master, origin/HEAD, master) Add linux-next specific files for 20241001
> > > >
> > > > Again it took two attempts to trigger the bug.
> > > >
> > > > Bert Karwatzki
> > > >
> > >

[snip]

OK sorry to keep on updating so quick, but been busy testing out the fix.

This version passes all tests so shooould work generally.

I'll do a proper write-up, etc. but generally it's because we can have a
condition where one node is empty other than the end node that is to be
overwritten, but we weren't being careful about that on a spanning store,
so were merging this AND writing in the new entry resulting in the observed
duplicate.

You need very specific circumstances to trigger this which the MAP_FIXED
series + steam seemed to provide :)

Please give this a try and let me know how it works.

Huge thanks for all your help throughout this! :)

----8<----
>From 3d01e0f97a2a62992b5f2bd466908628ae72da95 Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Date: Fri, 4 Oct 2024 15:18:58 +0100
Subject: [PATCH] fix v3

This now passes all maple tree tests, so should be good to go...

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
---
 lib/maple_tree.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 37abf0fe380b..4c0bb56a3142 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -3527,6 +3527,7 @@ static bool mas_wr_walk(struct ma_wr_state *wr_mas)
 	return true;
 }

+// Return whether the node actually contains entries at or greater than wr_mas->mas->index.
 static bool mas_wr_walk_index(struct ma_wr_state *wr_mas)
 {
 	struct ma_state *mas = wr_mas->mas;
@@ -3535,8 +3536,11 @@ static bool mas_wr_walk_index(struct ma_wr_state *wr_mas)
 		mas_wr_walk_descend(wr_mas);
 		wr_mas->content = mas_slot_locked(mas, wr_mas->slots,
 						  mas->offset);
-		if (ma_is_leaf(wr_mas->type))
-			return true;
+		if (ma_is_leaf(wr_mas->type)) {
+			unsigned long pivot = wr_mas->pivots[mas->offset];
+
+			return pivot == 0 || mas->index <= pivot;
+		}
 		mas_wr_walk_traverse(wr_mas);

 	}
@@ -3696,6 +3700,7 @@ static noinline void mas_wr_spanning_store(struct ma_wr_state *wr_mas)
 	struct maple_big_node b_node;
 	struct ma_state *mas;
 	unsigned char height;
+	bool r_populated;

 	/* Left and Right side of spanning store */
 	MA_STATE(l_mas, NULL, 0, 0);
@@ -3737,7 +3742,7 @@ static noinline void mas_wr_spanning_store(struct ma_wr_state *wr_mas)
 		r_mas.last++;

 	r_mas.index = r_mas.last;
-	mas_wr_walk_index(&r_wr_mas);
+	r_populated = mas_wr_walk_index(&r_wr_mas);
 	r_mas.last = r_mas.index = mas->last;

 	/* Set up left side. */
@@ -3759,9 +3764,12 @@ static noinline void mas_wr_spanning_store(struct ma_wr_state *wr_mas)

 	memset(&b_node, 0, sizeof(struct maple_big_node));
 	/* Copy l_mas and store the value in b_node. */
+
 	mas_store_b_node(&l_wr_mas, &b_node, l_mas.end);
+
 	/* Copy r_mas into b_node. */
-	if (r_mas.offset <= r_mas.end)
+
+	if (r_populated && r_mas.offset <= r_mas.end)
 		mas_mab_cp(&r_mas, r_mas.offset, r_mas.end,
 			   &b_node, b_node.b_end + 1);
 	else
--
2.46.2