On Thu, Jul 31, 2025 at 02:56:25PM +0200, David Hildenbrand wrote:
On 31.07.25 14:37, Sasha Levin wrote:
On Tue, Jul 08, 2025 at 05:42:16PM +0200, David Hildenbrand wrote:
On 08.07.25 17:33, Sasha Levin wrote:
On Tue, Jul 08, 2025 at 05:10:44PM +0200, David Hildenbrand wrote:
On 01.07.25 02:57, Andrew Morton wrote:
On Sun, 29 Jun 2025 23:19:58 -0400 Sasha Levin sashal@kernel.org wrote:
>When handling non-swap entries in move_pages_pte(), the error handling >for entries that are NOT migration entries fails to unmap the page table >entries before jumping to the error handling label. > >This results in a kmap/kunmap imbalance which on CONFIG_HIGHPTE systems >triggers a WARNING in kunmap_local_indexed() because the kmap stack is >corrupted. > >Example call trace on ARM32 (CONFIG_HIGHPTE enabled): > WARNING: CPU: 1 PID: 633 at mm/highmem.c:622 kunmap_local_indexed+0x178/0x17c > Call trace: > kunmap_local_indexed from move_pages+0x964/0x19f4 > move_pages from userfaultfd_ioctl+0x129c/0x2144 > userfaultfd_ioctl from sys_ioctl+0x558/0xd24 > >The issue was introduced with the UFFDIO_MOVE feature but became more >frequent with the addition of guard pages (commit 7c53dfbdb024 ("mm: add >PTE_MARKER_GUARD PTE marker")) which made the non-migration entry code >path more commonly executed during userfaultfd operations. > >Fix this by ensuring PTEs are properly unmapped in all non-swap entry >paths before jumping to the error handling label, not just for migration >entries.
I don't get it.
>--- a/mm/userfaultfd.c >+++ b/mm/userfaultfd.c >@@ -1384,14 +1384,15 @@ static int move_pages_pte(struct mm_struct *mm, pmd_t *dst_pmd, pmd_t *src_pmd, > entry = pte_to_swp_entry(orig_src_pte); > if (non_swap_entry(entry)) { >+ pte_unmap(src_pte); >+ pte_unmap(dst_pte); >+ src_pte = dst_pte = NULL; > if (is_migration_entry(entry)) { >- pte_unmap(src_pte); >- pte_unmap(dst_pte); >- src_pte = dst_pte = NULL; > migration_entry_wait(mm, src_pmd, src_addr); > err = -EAGAIN; >- } else >+ } else { > err = -EFAULT; >+ } > goto out;
where we have
out: ... if (dst_pte) pte_unmap(dst_pte); if (src_pte) pte_unmap(src_pte);
AI slop?
Nah, this one is sadly all me :(
Haha, sorry :P
So as I was getting nowhere with this, I asked AI to help me :)
If you're not interested in reading LLM generated code, feel free to stop reading now...
After it went over the logs, and a few prompts to point it the right way, it ended up generating a patch (below) that made sense, and fixed the warning that LKFT was being able to trigger.
If anyone who's more familiar with the code than me (and the AI) agrees with the patch and ways to throw their Reviewed-by, I'll send out the patch.
Seems to check out for me. In particular, out pte_unmap() everywhere else in that function (and mremap.c:move_ptes) are ordered properly.
Even if it would not fix the issue, it would be a cleanup :)
Acked-by: David Hildenbrand david@redhat.com
Thanks for the review!
I'll send this patch out properly.