Re: [PATCH v15 05/11] mm/hmm: migrate collection in HMM pagewalk - pte level

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mm: livelock in migrate_vma_split_folio() due to folio_get() before folio_lock()
- [High] mm: crash in migrate_vma_split_folio() due to unvalidated non-folio compound page
- [High] mm: valid pages marked with HMM_PFN_ERROR after transient split failure
--

commit 9b874173389a5794c12372b3f46c3c2b4f634054
Author: Mika Penttilä <mpenttil@redhat.com>

mm/hmm: migrate collection in HMM pagewalk - pte level

Implement hmm_vma_handle_migrate_prepare() for migration decisions and
migrate_vma_split_folio() for splitting pte mapped large folios during
HMM pagewalks.

> diff --git a/mm/hmm.c b/mm/hmm.c
> index 25f537c87b5f2..a60b669630496 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c

[ ... ]

> +static int migrate_vma_split_folio(struct folio *folio,
> +				   struct page *fault_page,
> +				   struct hmm_vma_walk *hmm_vma_walk,
> +				   pte_t *ptep)
> +{
> +	int ret;
> +	struct folio *fault_folio = fault_page ? page_folio(fault_page) : NULL;
> +	struct folio *new_fault_folio = NULL;
> +
> +	if (folio != fault_folio)
> +		folio_get(folio);
> +
> +	pte_unmap_unlock(ptep, hmm_vma_walk->ptl);
> +	hmm_vma_walk->ptelocked = false;
> +
> +	if (folio != fault_folio)
> +		folio_lock(folio);

[Severity: High]
Can this cause a livelock when splitting the folio?

If multiple threads concurrently fault on the same shared large folio and
enter migrate_vma_split_folio(), they will all acquire a reference via
folio_get() before blocking on folio_lock(). This inflates the refcount,
causing split_folio() to fail with -EAGAIN. Retrying this sequence creates
a permanent livelock.

[ ... ]

> +static int hmm_vma_handle_migrate_prepare(const struct mm_walk *walk,
> +					  pmd_t *pmdp,
> +					  pte_t *ptep,
> +					  unsigned long addr,
> +					  unsigned long *hmm_pfn,
> +					  bool *unmapped)
> +{

[ ... ]

> +		folio = page ? page_folio(page) : NULL;
> +		if (folio && folio_test_large(folio)) {
> +			int ret;
> +
> +			ret = migrate_vma_split_folio(folio,
> +						      migrate->fault_page,
> +						      hmm_vma_walk,
> +						      ptep);
> +			if (ret)
> +				goto out_error;
> +			return -EAGAIN;
> +		}
> +
> +		writable = pte_write(pte);
> +	}
> +
> +	if (!page || !page->mapping)
> +		goto out;

[Severity: High]
Does this code operate on uninitialized folio metadata if a driver maps an
uninitialized compound page to userspace?

If a driver allocates a compound page without folio initialization and maps
it, hmm_range_fault() walking the VMA will see PG_head set, and
folio_test_large() will succeed. Because the !page->mapping validation check
is deferred until after the split attempt, this passes the uninitialized
compound page to migrate_vma_split_folio(), which can crash when calling
folio_lock() and split_folio().

[ ... ]

> +out:
>  	return 0;
> +out_error:
> +	return -EFAULT;
>  }

[Severity: High]
Can a transient split failure result in valid pages being falsely marked as
inaccessible?

If migrate_vma_split_folio() fails due to a transient refcount elevation,
hmm_vma_handle_migrate_prepare() returns -EFAULT. When hmm_vma_walk_pmd()
catches this non -EAGAIN error, it breaks the loop and calls
hmm_pfns_fill(..., HMM_PFN_ERROR) for the entire remainder of the PMD.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924065313.899730-1-mpenttil@redhat.com?part=5