On Thu, Jul 31, 2025 at 7:24 AM Suren Baghdasaryan surenb@google.com wrote:
On Thu, Jul 31, 2025 at 12:35 AM Lokesh Gidra lokeshgidra@google.com wrote:
On Wed, Jul 30, 2025 at 6:58 PM Hillf Danton hdanton@sina.com wrote:
#syz test
When UFFDIO_MOVE is used with UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES and it encounters a non-present THP, it fails to properly recognize an unmapped hole and tries to access a non-existent folio, resulting in a crash. Add a check to skip non-present THPs.
Thanks Suren for promptly addressing this issue.
Fixes: adef440691ba ("userfaultfd: UFFDIO_MOVE uABI") Reported-by: syzbot+b446dbe27035ef6bd6c2@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/68794b5c.a70a0220.693ce.0050.GAE@google.com/ Signed-off-by: Suren Baghdasaryan surenb@google.com Cc: stable@vger.kernel.org
mm/userfaultfd.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c index cbed91b09640..60be8080ddd0 100644 --- a/mm/userfaultfd.c +++ b/mm/userfaultfd.c @@ -1818,27 +1818,35 @@ ssize_t move_pages(struct userfaultfd_ctx *ctx, unsigned long dst_start,
ptl = pmd_trans_huge_lock(src_pmd, src_vma); if (ptl) {
/* Check if we can move the pmd without splitting it. */if (move_splits_huge_pmd(dst_addr, src_addr, src_start + len) ||!pmd_none(dst_pmdval)) {struct folio *folio = pmd_folio(*src_pmd);
if (pmd_present(*src_pmd) || is_pmd_migration_entry(*src_pmd)) {/* Check if we can move the pmd without splitting it. */if (move_splits_huge_pmd(dst_addr, src_addr, src_start + len) ||!pmd_none(dst_pmdval)) {if (pmd_present(*src_pmd)) {struct folio *folio = pmd_folio(*src_pmd);if (!folio || (!is_huge_zero_folio(folio) &&!PageAnonExclusive(&folio->page))) {spin_unlock(ptl);err = -EBUSY;break;}}
if (!folio || (!is_huge_zero_folio(folio) &&!PageAnonExclusive(&folio->page))) { spin_unlock(ptl);err = -EBUSY;break;
split_huge_pmd(src_vma, src_pmd, src_addr);/* The folio will be split by move_pages_pte() */continue; }err = move_pages_huge_pmd(mm, dst_pmd, src_pmd,dst_pmdval, dst_vma, src_vma,dst_addr, src_addr);} else {/* nothing to do to move a hole */ spin_unlock(ptl);
split_huge_pmd(src_vma, src_pmd, src_addr);/* The folio will be split by move_pages_pte() */continue;
err = 0;I think we need to act here depending on whether UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES is set or not.
Hmm, yes, I think you are right. I thought we would bail out earlier if !UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES but I think it's possible to get here if the PMD was established earlier but then unmapped.
That makes sense too. My thinking was that the !UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES check above is only when !src_pmd, which means the src pmd-page itself isn't present. However, the case where pmd-page is present, but the pmd entry is not; continuing skipping the hole even when user cannot tolerate src-holes would break the userspace logic.
err = (mode & UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES) ? 0 : -ENOENT;Also, IMO, the step_size in this case should be the minimum of remaining length and HPAGE_PMD_SIZE.
Ah, ok. I think it matters only for incrementing "moved" correctly because otherwise the functionality is the same.
Right. Returning an incorrect "moved" value to userspace can break the logic, possibly causing data corruption.
}
err = move_pages_huge_pmd(mm, dst_pmd, src_pmd,dst_pmdval, dst_vma, src_vma,dst_addr, src_addr); step_size = HPAGE_PMD_SIZE; } else { if (pmd_none(*src_pmd)) {I have a related question/doubt: why do we populate the page-table hierarchy on the src side [1] (and then also at line 1857) when a hole is found? IMHO, it shouldn't be needed. Depending on whether UFFDIO_MOVE_MODE_ALLOW_SRC_HOLES is set or not, it should either return -ENOENT, or continue past the hole. Please correct me if I'm wrong.
I thought about that too. I think it's done to simplify the logic. This way we can treat the cases when PMD was never allocated and when PMD was allocated, mapped and then unmapped the same way.
Makes sense. Thanks for clarifying.
[1] https://elixir.bootlin.com/linux/v6.16/source/mm/userfaultfd.c#L1797
base-commit: 01da54f10fddf3b01c5a3b80f6b16bbad390c302
2.50.1.552.g942d659e1b-goog