1. Patchew
  2. linux
  3. View series

[PATCH] mm/userfaultfd: fix kmap_local LIFO ordering for CONFIG_HIGHPTE

Sasha Levin posted 1 patch 2 months ago 
mm/userfaultfd.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
[PATCH] mm/userfaultfd: fix kmap_local LIFO ordering for CONFIG_HIGHPTE
Posted by Sasha Levin 2 months ago 
With CONFIG_HIGHPTE on 32-bit ARM, move_pages_pte() maps PTE pages using
kmap_local_page(), which requires unmapping in Last-In-First-Out order.

The current code maps dst_pte first, then src_pte, but unmaps them in
the same order (dst_pte, src_pte), violating the LIFO requirement.
This causes the warning in kunmap_local_indexed():

  WARNING: CPU: 0 PID: 604 at mm/highmem.c:622 kunmap_local_indexed+0x178/0x17c
  addr \!= __fix_to_virt(FIX_KMAP_BEGIN + idx)

Fix this by reversing the unmap order to respect LIFO ordering.

This issue follows the same pattern as similar fixes:
- commit eca6828403b8 ("crypto: skcipher - fix mismatch between mapping and unmapping order")
- commit 8cf57c6df818 ("nilfs2: eliminate staggered calls to kunmap in nilfs_rename")

Both of which addressed the same fundamental requirement that kmap_local
operations must follow LIFO ordering.

Fixes: adef440691ba ("userfaultfd: UFFDIO_MOVE uABI")
Co-developed-by: Claude claude-opus-4-20250514
Signed-off-by: Sasha Levin <sashal@kernel.org>
Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
---
 mm/userfaultfd.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 8253978ee0fb..bf7a57ea71e0 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -1453,10 +1453,15 @@ static int move_pages_pte(struct mm_struct *mm, pmd_t *dst_pmd, pmd_t *src_pmd,
 		folio_unlock(src_folio);
 		folio_put(src_folio);
 	}
-	if (dst_pte)
-		pte_unmap(dst_pte);
+	/*
+	 * Unmap in reverse order (LIFO) to maintain proper kmap_local
+	 * index ordering when CONFIG_HIGHPTE is enabled. We mapped dst_pte
+	 * first, then src_pte, so we must unmap src_pte first, then dst_pte.
+	 */
 	if (src_pte)
 		pte_unmap(src_pte);
+	if (dst_pte)
+		pte_unmap(dst_pte);
 	mmu_notifier_invalidate_range_end(&range);
 	if (si)
 		put_swap_device(si);
-- 
2.39.5
Re: [PATCH] mm/userfaultfd: fix kmap_local LIFO ordering for CONFIG_HIGHPTE
Posted by Andrew Morton 2 months ago 
On Thu, 31 Jul 2025 10:44:31 -0400 Sasha Levin <sashal@kernel.org> wrote:

> With CONFIG_HIGHPTE on 32-bit ARM, move_pages_pte() maps PTE pages using
> kmap_local_page(), which requires unmapping in Last-In-First-Out order.
> 
> The current code maps dst_pte first, then src_pte, but unmaps them in
> the same order (dst_pte, src_pte), violating the LIFO requirement.
> This causes the warning in kunmap_local_indexed():
> 
>   WARNING: CPU: 0 PID: 604 at mm/highmem.c:622 kunmap_local_indexed+0x178/0x17c
>   addr \!= __fix_to_virt(FIX_KMAP_BEGIN + idx)
> 
> Fix this by reversing the unmap order to respect LIFO ordering.
> 
> This issue follows the same pattern as similar fixes:
> - commit eca6828403b8 ("crypto: skcipher - fix mismatch between mapping and unmapping order")
> - commit 8cf57c6df818 ("nilfs2: eliminate staggered calls to kunmap in nilfs_rename")
> 
> Both of which addressed the same fundamental requirement that kmap_local
> operations must follow LIFO ordering.
> 
> Fixes: adef440691ba ("userfaultfd: UFFDIO_MOVE uABI")
> Co-developed-by: Claude claude-opus-4-20250514

Well this is innovative.  I doubt if Co-developed-by: is appropriate
for this (where's Claude's Signed-off-by:?)

I'd support creating a new changelog tag for this case.

And really, if AI was recruited in developing a kernel patch, it would
be helpful if the changelog were to have a paragraph describing just
how the AI assist was used.  At least, until everyone knows all about
this?  You probably already have a presentation or a web page, so
adding a link to that would suffice, thanks.
Re: [PATCH] mm/userfaultfd: fix kmap_local LIFO ordering for CONFIG_HIGHPTE
Posted by Sasha Levin 2 months ago 
On Fri, Aug 01, 2025 at 02:11:01PM -0700, Andrew Morton wrote:
>On Thu, 31 Jul 2025 10:44:31 -0400 Sasha Levin <sashal@kernel.org> wrote:
>
>> With CONFIG_HIGHPTE on 32-bit ARM, move_pages_pte() maps PTE pages using
>> kmap_local_page(), which requires unmapping in Last-In-First-Out order.
>>
>> The current code maps dst_pte first, then src_pte, but unmaps them in
>> the same order (dst_pte, src_pte), violating the LIFO requirement.
>> This causes the warning in kunmap_local_indexed():
>>
>>   WARNING: CPU: 0 PID: 604 at mm/highmem.c:622 kunmap_local_indexed+0x178/0x17c
>>   addr \!= __fix_to_virt(FIX_KMAP_BEGIN + idx)
>>
>> Fix this by reversing the unmap order to respect LIFO ordering.
>>
>> This issue follows the same pattern as similar fixes:
>> - commit eca6828403b8 ("crypto: skcipher - fix mismatch between mapping and unmapping order")
>> - commit 8cf57c6df818 ("nilfs2: eliminate staggered calls to kunmap in nilfs_rename")
>>
>> Both of which addressed the same fundamental requirement that kmap_local
>> operations must follow LIFO ordering.
>>
>> Fixes: adef440691ba ("userfaultfd: UFFDIO_MOVE uABI")
>> Co-developed-by: Claude claude-opus-4-20250514
>
>Well this is innovative.  I doubt if Co-developed-by: is appropriate
>for this (where's Claude's Signed-off-by:?)

Claude (or any other AI) can't legally sign off on code :)

>I'd support creating a new changelog tag for this case.

This is in the context of a proposal on workflows@:
https://lore.kernel.org/workflows/20250728105634.GF787@pendragon.ideasonboard.com/T/#t

The Co-developed-by: usage wasn't my proposal, but it looked like the
majority of folks were okay with it.

Input is definitely welcome!

>And really, if AI was recruited in developing a kernel patch, it would
>be helpful if the changelog were to have a paragraph describing just
>how the AI assist was used.  At least, until everyone knows all about
>this?  You probably already have a presentation or a web page, so
>adding a link to that would suffice, thanks.

Kees actually has a good writeup about his experience with AI tooling
here: https://hachyderm.io/@kees/114907228284590439 , my experience is
fairly similar.

Kees logged his prompts as part of the patch he sent in
(https://lore.kernel.org/lkml/20250724080756.work.741-kees@kernel.org/)
which was interesting, but I didn't see much value in doing that beyond
the demo purposes as this is not really reproducible.

-- 
Thanks,
Sasha
Re: [PATCH] mm/userfaultfd: fix kmap_local LIFO ordering for CONFIG_HIGHPTE
Posted by David Hildenbrand 2 months ago 
On 01.08.25 23:56, Sasha Levin wrote:
> On Fri, Aug 01, 2025 at 02:11:01PM -0700, Andrew Morton wrote:
>> On Thu, 31 Jul 2025 10:44:31 -0400 Sasha Levin <sashal@kernel.org> wrote:
>>
>>> With CONFIG_HIGHPTE on 32-bit ARM, move_pages_pte() maps PTE pages using
>>> kmap_local_page(), which requires unmapping in Last-In-First-Out order.
>>>
>>> The current code maps dst_pte first, then src_pte, but unmaps them in
>>> the same order (dst_pte, src_pte), violating the LIFO requirement.
>>> This causes the warning in kunmap_local_indexed():
>>>
>>>    WARNING: CPU: 0 PID: 604 at mm/highmem.c:622 kunmap_local_indexed+0x178/0x17c
>>>    addr \!= __fix_to_virt(FIX_KMAP_BEGIN + idx)
>>>
>>> Fix this by reversing the unmap order to respect LIFO ordering.
>>>
>>> This issue follows the same pattern as similar fixes:
>>> - commit eca6828403b8 ("crypto: skcipher - fix mismatch between mapping and unmapping order")
>>> - commit 8cf57c6df818 ("nilfs2: eliminate staggered calls to kunmap in nilfs_rename")
>>>
>>> Both of which addressed the same fundamental requirement that kmap_local
>>> operations must follow LIFO ordering.
>>>
>>> Fixes: adef440691ba ("userfaultfd: UFFDIO_MOVE uABI")
>>> Co-developed-by: Claude claude-opus-4-20250514
>>
>> Well this is innovative.  I doubt if Co-developed-by: is appropriate
>> for this (where's Claude's Signed-off-by:?)
> 
> Claude (or any other AI) can't legally sign off on code :)

I think we need a different tag than Co-developed-by. Avocado [1] seems 
to use

	Assisted-by:

Which I prefer over things like Generated-by o co-developed-by:

[1] 
https://avocado-framework.readthedocs.io/en/latest/guides/contributor/chapters/ai_policy.html

-- 
Cheers,

David / dhildenb
Re: [PATCH] mm/userfaultfd: fix kmap_local LIFO ordering for CONFIG_HIGHPTE
Posted by Andrew Morton 1 month, 2 weeks ago 
On Fri, 1 Aug 2025 17:56:25 -0400 Sasha Levin <sashal@kernel.org> wrote:

> >> Fixes: adef440691ba ("userfaultfd: UFFDIO_MOVE uABI")
> >> Co-developed-by: Claude claude-opus-4-20250514
> >
> >Well this is innovative.  I doubt if Co-developed-by: is appropriate
> >for this (where's Claude's Signed-off-by:?)
> 
> Claude (or any other AI) can't legally sign off on code :)
> 
> >I'd support creating a new changelog tag for this case.
> 
> This is in the context of a proposal on workflows@:
> https://lore.kernel.org/workflows/20250728105634.GF787@pendragon.ideasonboard.com/T/#t
> 
> The Co-developed-by: usage wasn't my proposal, but it looked like the
> majority of folks were okay with it.
> 

I need to do something about this and the discussion over on
ksummit@lists.linux.dev has been as beer-addled as one would expect.

Oh well.  I guess for now we can welcome Claude to the kernel
development team.
Re: [PATCH] mm/userfaultfd: fix kmap_local LIFO ordering for CONFIG_HIGHPTE
Posted by Sasha Levin 1 month, 2 weeks ago 
On Wed, Aug 20, 2025 at 09:24:23PM -0700, Andrew Morton wrote:
>On Fri, 1 Aug 2025 17:56:25 -0400 Sasha Levin <sashal@kernel.org> wrote:
>
>> >> Fixes: adef440691ba ("userfaultfd: UFFDIO_MOVE uABI")
>> >> Co-developed-by: Claude claude-opus-4-20250514
>> >
>> >Well this is innovative.  I doubt if Co-developed-by: is appropriate
>> >for this (where's Claude's Signed-off-by:?)
>>
>> Claude (or any other AI) can't legally sign off on code :)
>>
>> >I'd support creating a new changelog tag for this case.
>>
>> This is in the context of a proposal on workflows@:
>> https://lore.kernel.org/workflows/20250728105634.GF787@pendragon.ideasonboard.com/T/#t
>>
>> The Co-developed-by: usage wasn't my proposal, but it looked like the
>> majority of folks were okay with it.
>>
>
>I need to do something about this and the discussion over on
>ksummit@lists.linux.dev has been as beer-addled as one would expect.
>
>Oh well.  I guess for now we can welcome Claude to the kernel
>development team.

FWIW, I've switched the new RFC to use Assisted-by: as suggested by yourself
and David. If you'd prefer, I can resend it with the Assisted-by tag instead.

-- 
Thanks,
Sasha

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.