Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] mm: Use-After-Free in __migrate_vma_split_folio on unlocked tail page --
commit 303a19fde6deb0a6633524827ecf0a51aa867674 Author: Mika Penttilä <[email protected]> mm/hmm: migrate collection in HMM pagewalk - pmd level This commit implements hmm_vma_handle_migrate_prepare_pmd() to handle migration decisions and split folios for HMM pagewalk-based migration. It also refactors the folio splitting path by separating it into __migrate_vma_split_folio() to fix extra refcount and unlock issues. > diff --git a/mm/hmm.c b/mm/hmm.c > index a60b669630496..ea77429c5178b 100644 > --- a/mm/hmm.c > +++ b/mm/hmm.c > @@ -492,31 +492,19 @@ static int hmm_vma_handle_absent_pmd(struct mm_walk > *walk, unsigned long start, > > #ifdef CONFIG_DEVICE_MIGRATION > /** > - * migrate_vma_split_folio() - Helper function to split a THP folio > + * __migrate_vma_split_folio() - split a folio and move the lock/ref to the > + * order-0 folio backing @fault_page after the split > * @folio: the folio to split > - * @fault_page: struct page associated with the fault if any > - * @hmm_vma_walk: walk in progress > - * @ptep: pte_t * for unmap and unlock ptl > * > - * Returns 0 on success > + * Returns 0 on success. > */ > -static int migrate_vma_split_folio(struct folio *folio, > - struct page *fault_page, > - struct hmm_vma_walk *hmm_vma_walk, > - pte_t *ptep) > +static int __migrate_vma_split_folio(struct folio *folio, > + struct page *fault_page) > { > - 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); > + int ret; > > ret = split_folio(folio); > if (ret) { [Severity: High] Can this lead to a use-after-free on an unlocked tail page? If split_folio() successfully splits a THP where the fault page is a tail page, it returns with the head page locked but unfreezes and unlocks all the tail pages. Later in this function, the code directly acquires a reference and lock on the new tail folio: if (new_fault_folio != fault_folio) { folio_get(new_fault_folio); folio_lock(new_fault_folio); } Since the PMD lock was dropped before calling __migrate_vma_split_folio() and mmap_lock is only held in read mode, could a concurrent thread (like memory reclaim) isolate, unmap, and free the unlocked tail page to the buddy allocator before folio_get() is called? Should this use folio_try_get() to safely acquire a reference to the speculatively accessed tail folio? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
