| 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
| ||
|
Message-ID: <6bd5ffe2-8f28-497e-9092-085e5d1cbc1f@redhat.com>
Date: Mon, 18 Aug 2025 14:54:29 +0200
From: David Hildenbrand <david@...hat.com>
To: Harry Yoo <harry.yoo@...cle.com>,
syzbot <syzbot+4d9a13f0797c46a29e42@...kaller.appspotmail.com>,
Peter Xu <peterx@...hat.com>
Cc: Liam.Howlett@...cle.com, akpm@...ux-foundation.org, jannh@...gle.com,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
lorenzo.stoakes@...cle.com, pfalcato@...e.de,
syzkaller-bugs@...glegroups.com, vbabka@...e.cz,
Ryan Roberts <ryan.roberts@....com>,
Mikołaj Lenczewski <miko.lenczewski@....com>
Subject: Re: [syzbot] [mm?] WARNING in move_page_tables
On 13.08.25 14:20, Harry Yoo wrote:
> On Tue, Aug 12, 2025 at 02:56:35PM -0700, syzbot wrote:
>> Hello,
>>
>> syzbot found the following issue on:
>>
>> HEAD commit: 53e760d89498 Merge tag 'nfsd-6.17-1' of git://git.kernel.o..
>> git tree: upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=165fe9a2580000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=f9319a42cfb3bf57
>> dashboard link: https://syzkaller.appspot.com/bug?extid=4d9a13f0797c46a29e42
>> compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14172842580000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15b04c34580000
>>
>> Downloadable assets:
>> disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-53e760d8.raw.xz
>> vmlinux: https://storage.googleapis.com/syzbot-assets/584b4139c7e3/vmlinux-53e760d8.xz
>> kernel image: https://storage.googleapis.com/syzbot-assets/4d2474607300/bzImage-53e760d8.xz
>>
>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>> Reported-by: syzbot+4d9a13f0797c46a29e42@...kaller.appspotmail.com
>
> [Cc'ing Ryan, Mikołaj, David and Peter]
>
> I was able to reliably reproduce this (with the reproducer provided
> by syzbot) and performed bisection.
>
> The first bad commit is 0cef0bb836e mm: clear uffd-wp PTE/PMD state on
> mremap(), which was introduced in v6.13.
>
Okay, so we're hitting the
if (WARN_ON_ONCE(!pmd_none(*new_pmd)))
return false;
in move_normal_pmd().
Given that the reproducer involves allocation fault injection during move_page_tables(),
I assume we run into this warning when we are trying to restore our previous state,
so when we call move_page_tables() the
second time from copy_vma_and_data().
Something when moving stuff back after a failed PTE table allocation is broken.
Ah, maybe I know what happens.
When we move the first time, we check "vma_has_uffd_without_event_remap(orig_vma)"
and see that "yes, this thing has uffd" and decide to move PTE level
When we move back, we check "vma_has_uffd_without_event_remap(new_vma)" and see
that "no, this thing does not have uffd" and decide to move PMD level.
But the original PTE table is still there ...
As a side-note: It's confusing to call vma_has_uffd_without_event_remap() to make
a decision during mremap to handle WP, when WP might not even be active.
We should likely slap in a uffd-wp check on the VMA as a follow-up cleanup.
#syz test
diff --git a/mm/mremap.c b/mm/mremap.c
index 33b642076205d..a9730f4373b77 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -323,6 +323,26 @@ static inline bool arch_supports_page_table_move(void)
}
#endif
+static inline bool uffd_supports_page_table_move(struct pagetable_move_control *pmc)
+{
+ /*
+ * If we are moving VMAs that have uffd-wp registered but with
+ * remap events disabled (new VMA will not be registered with uffd), we
+ * need to ensure that the uffd-wp state is cleared from all pgtables.
+ * This means recursing into lower page tables in move_page_tables().
+ *
+ * We setup the uffd-wp context on the new VMA after moving the page
+ * tables succeeded, so checking the old VMA looks reasonable. However,
+ * when we have to recover from a failed page table move attempt, we
+ * get called with inverse VMAs. Recursing into lower page tables during
+ * the original move but not during the recovery move will cause
+ * trouble, because we run into already-existing page tables during
+ * the recovery phase. Consequently, check both VMAs.
+ */
+ return !vma_has_uffd_without_event_remap(pmc->old) &&
+ !vma_has_uffd_without_event_remap(pmc->new);
+}
+
#ifdef CONFIG_HAVE_MOVE_PMD
static bool move_normal_pmd(struct pagetable_move_control *pmc,
pmd_t *old_pmd, pmd_t *new_pmd)
@@ -335,6 +355,8 @@ static bool move_normal_pmd(struct pagetable_move_control *pmc,
if (!arch_supports_page_table_move())
return false;
+ if (!uffd_supports_page_table_move(pmc))
+ return false;
/*
* The destination pmd shouldn't be established, free_pgtables()
* should have released it.
@@ -361,15 +383,6 @@ static bool move_normal_pmd(struct pagetable_move_control *pmc,
if (WARN_ON_ONCE(!pmd_none(*new_pmd)))
return false;
- /* If this pmd belongs to a uffd vma with remap events disabled, we need
- * to ensure that the uffd-wp state is cleared from all pgtables. This
- * means recursing into lower page tables in move_page_tables(), and we
- * can reuse the existing code if we simply treat the entry as "not
- * moved".
- */
- if (vma_has_uffd_without_event_remap(vma))
- return false;
-
/*
* We don't have to worry about the ordering of src and dst
* ptlocks because exclusive mmap_lock prevents deadlock.
@@ -418,6 +431,8 @@ static bool move_normal_pud(struct pagetable_move_control *pmc,
if (!arch_supports_page_table_move())
return false;
+ if (!uffd_supports_page_table_move(pmc))
+ return false;
/*
* The destination pud shouldn't be established, free_pgtables()
* should have released it.
@@ -425,15 +440,6 @@ static bool move_normal_pud(struct pagetable_move_control *pmc,
if (WARN_ON_ONCE(!pud_none(*new_pud)))
return false;
- /* If this pud belongs to a uffd vma with remap events disabled, we need
- * to ensure that the uffd-wp state is cleared from all pgtables. This
- * means recursing into lower page tables in move_page_tables(), and we
- * can reuse the existing code if we simply treat the entry as "not
- * moved".
- */
- if (vma_has_uffd_without_event_remap(vma))
- return false;
-
/*
* We don't have to worry about the ordering of src and dst
* ptlocks because exclusive mmap_lock prevents deadlock.
--
2.50.1
--
Cheers
David / dhildenb
Powered by blists - more mailing lists