mm/mprotect: Fix maple tree start address in do_mprotect_pkey()

Commit Message

Liam R. Howlett Aug. 25, 2022, 8:30 p.m. UTC

Use the untagged_addr() instead of the address passed into the function.

Fixes: 3338b715d25d (mm/mprotect: use maple tree navigation instead of vma linked list)
Signed-off-by: Liam R. Howlett <[e-mail]>
---
 mm/mprotect.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Andrew Morton Aug. 26, 2022, 1:06 a.m. UTC | #1

On Thu, 25 Aug 2022 20:30:24 +0000 Liam Howlett <[e-mail]> wrote:

> Use the untagged_addr() instead of the address passed into the function.

What are the runtime effects of this fix?

Liam R. Howlett Aug. 26, 2022, 3:27 a.m. UTC | #2

* Andrew Morton <[e-mail]> [220825 21:06]:
> On Thu, 25 Aug 2022 20:30:24 +0000 Liam Howlett <[e-mail]> wrote:
> 
> > Use the untagged_addr() instead of the address passed into the function.
> 
> What are the runtime effects of this fix?

Prior to my change to use the maple tree, the start address was changed
before calling find_vma() with the untagged_addr() version of start.  My
first change recorded the tagged address and searched on the incorrect
start location - which would have found the incorrect VMA.  This fix
will use the untagged_addr() as the start of the search as it was
before I changed the code at all.

Any penalty of calling untagged_addr() occurred regardless of the
version that was used.  The search of the maple tree would have also
occurred in both versions - just at the wrong location before this fix.
I expect that the execution time would be equal as the search on the
tagged address would have either returned a VMA at start, or the VMA in
the next slot in the maple tree node - probably immeasurably slower
since the data is very likely already in the CPU cache, but I don't have
hard data to say either way.  I can look into a benchmark to measure the
difference between both working versions, but I don't have an arm64
native target so it will be emulated.

Patch

diff --git a/mm/mprotect.c b/mm/mprotect.c
index f2b9b1da9083..3c79796be65e 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -675,7 +675,7 @@  static int do_mprotect_pkey(unsigned long start, size_t len,
 	const bool rier = (current->personality & READ_IMPLIES_EXEC) &&
 				(prot & PROT_READ);
 	struct mmu_gather tlb;
-	MA_STATE(mas, &current->mm->mm_mt, start, start);
+	MA_STATE(mas, &current->mm->mm_mt, 0, 0);
 
 	start = untagged_addr(start);
 
@@ -707,6 +707,7 @@  static int do_mprotect_pkey(unsigned long start, size_t len,
 	if ((pkey != -1) && !mm_pkey_is_allocated(current->mm, pkey))
 		goto out;
 
+	mas_set(&mas, start);
 	vma = mas_find(&mas, ULONG_MAX);
 	error = -ENOMEM;
 	if (!vma)