Re: [PATCH v2 06/13] mm/migrate_device: don't let go of the fault page

[Balbir Singh <balbirs@nvidia.com>]

On Fri, Nov 13, 2026 at 09:45:24PM +0000, Lorenzo Stoakes (ARM) wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> When migrate_vma_split_folio() has split the folio of the fault page,
> the lock and the reference which the fault handler has taken are at the
> former head page.
> 
> It then moves them to the folio which now holds the fault page, by
> taking a reference on and locking that folio, then unlocking and putting
> the former head.
> 
> split_folio() has unlocked the new folio and has dropped its own
> reference on it, so until folio_get() nothing but the page table entry
> holds it.
> 
> Only the mmap lock is held for read, which does not prevent another thread
> from zapping the entry with MADV_DONTNEED.
> 
> If this happens the folio is freed which results in folio_get() taking a
> reference on a free device page, which the driver can hand out again at any
> time.
> 
> In addition, folio_lock() can sleep here with the lock of the head page
> held.
> 
> Utilise split_huge_page() which will leave the lock and reference at the
> fault page correctly.
> 
> Fixes: 022a12deda53 ("mm/migrate_device: handle partially mapped folios during collection")
> Cc: stable@vger.kernel.org
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> [ ljs: tweaked commit message ]
> Tested-by: Gregory Price (Meta) <gourry@gourry.net>
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
>  mm/migrate_device.c | 39 ++++++++++-----------------------------
>  1 file changed, 10 insertions(+), 29 deletions(-)
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 28ed706109da..ae795fdf36f4 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -87,39 +87,20 @@ static int migrate_vma_split_folio(struct folio *folio,
>  {
>  	int ret;
>  	struct folio *fault_folio = fault_page ? page_folio(fault_page) : NULL;
> -	struct folio *new_fault_folio = NULL;
> -
> -	if (folio != fault_folio)
> -		folio_lock(folio);
> -
> -	ret = split_folio(folio);
> -	if (ret) {
> -		if (folio != fault_folio) {
> -			folio_unlock(folio);
> -			folio_put(folio);
> -		}
> -		return ret;
> -	}
> -
> -	new_fault_folio = fault_page ? page_folio(fault_page) : NULL;
>  
>  	/*
> -	 * Ensure the lock is held on the correct
> -	 * folio after the split
> +	 * The lock and the reference of the fault handler stay with the
> +	 * folio which holds the fault page after the split.
>  	 */
> -	if (!new_fault_folio) {
> -		folio_unlock(folio);
> -		folio_put(folio);
> -	} else if (folio != new_fault_folio) {
> -		if (new_fault_folio != fault_folio) {
> -			folio_get(new_fault_folio);
> -			folio_lock(new_fault_folio);
> -		}
> -		folio_unlock(folio);
> -		folio_put(folio);
> -	}
> +	if (folio == fault_folio)
> +		return split_huge_page(fault_page);
>  
> -	return 0;
> +	folio_lock(folio);
> +	ret = split_folio(folio);
> +	folio_unlock(folio);
> +	folio_put(folio);
> +
> +	return ret;
>  }
>  
>  /** migrate_vma_collect_huge_pmd - collect THP pages without splitting the
>

Very subtle race

Reviewed-by: Balbir Singh <balbirs@nvidia.com>