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ä <[email protected]> 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/[email protected]?part=5
