Re: [PATCH 09/13] mm/migrate_device: fix use-after-free of the anon rmap

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] mm/migrate_device: mismatched folio unlock in
  migrate_device_range() [1]
--

commit 2aa24c588f51ec060d531e2046a8a14459e84b99
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

mm/migrate_device: fix use-after-free of the anon rmap

This commit fixes a use-after-free of the anonymous rmap during concurrent
device memory migration and munmap() by pinning the folio's anon_vma
before replacing PTEs and unpinning it after migration PTEs are removed.

> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index f5fb7f2d616a1..0f2a2832e0bd1 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
[ ... ]
> @@ -1391,6 +1414,7 @@ EXPORT_SYMBOL(migrate_vma_finalize);
>  
>  static unsigned long migrate_device_pfn_lock(unsigned long pfn)
>  {
> +	unsigned long src_pfn = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
>  	struct folio *folio;
>  
>  	folio = folio_get_nontail_page(pfn_to_page(pfn));
> @@ -1402,7 +1426,13 @@ static unsigned long migrate_device_pfn_lock(unsigned long pfn)
>  		return 0;
>  	}
>  
> -	return migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
> +	if (!pin_anon_rmap(folio, &src_pfn)) {
> +		folio_unlock(folio);
> +		folio_put(folio);
> +		return 0;
> +	}
> +
> +	return src_pfn;
>  }
>  
>  /**
> @@ -1438,6 +1468,7 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
>  		nr = folio_nr_pages(folio);
>  		if (nr > npages - i) {
>  			if (src_pfns[i] & MIGRATE_PFN_MIGRATE) {
> +				unpin_anon_rmap(folio, src_pfns[i]);
>  				folio_unlock(folio);
>  				folio_put(folio);
>  			}

[Severity: Critical]
This problem wasn't introduced by this patch, but can migrate_device_range()
(and migrate_device_pfns() below) unlock and put a stale folio pointer
[1] if the folio splits before migrate_device_pfn_lock() locks it?

Both loops read folio = page_folio(page) before acquiring a reference or
lock:

mm/migrate_device.c:migrate_device_range() {
	for (pfn = start, i = 0; i < npages; pfn++, i++) {
		struct page *page = pfn_to_page(pfn);
		struct folio *folio = page_folio(page);
		unsigned int nr = 1;

		src_pfns[i] = migrate_device_pfn_lock(pfn);
		nr = folio_nr_pages(folio);
}

If the compound page is split between page_folio(page) and
migrate_device_pfn_lock(), migrate_device_pfn_lock() locks and pins the
newly split folio, while the local folio variable still points to the old
compound head.  When nr > npages - i then triggers, unpin_anon_rmap(),
folio_unlock() and folio_put() operate on that unowned folio—triggering
VM_BUG_ON_FOLIO(!folio_test_locked(folio)) or use-after-free—while leaking
the lock, reference and anon_vma pin on the folio that
migrate_device_pfn_lock() actually locked.

[1] https://sashiko.dev/#/bug/linux-3f86966c-016e-475f-8285-11a7bf55a821

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-mason-migrate-et-al-fixes-v1-0-3d2454362e8d@kernel.org?part=9