Re: [PATCH v14 11/12] lib/test_hmm: add a new testcase for the migrate on fault

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: test_hmm: UAF due to concurrent unmaps during migration
--

commit 561fbddf4a910a84fca4e2648f745dbccb247398
Author: Mika Penttilä <mpenttil@redhat.com>

lib/test_hmm: add a new testcase for the migrate on fault

This commit enhances the hmm test driver (lib/test_hmm) with a new test
case for migrate on fault. It adds a new ioctl and test code to fault and
migrate anonymous memory to device private memory.

> diff --git a/lib/test_hmm.c b/lib/test_hmm.c
> index cd88e8177d0ab..b39522c05d832 100644
> --- a/lib/test_hmm.c
> +++ b/lib/test_hmm.c
[ ... ]
> @@ -382,7 +390,9 @@ static int dmirror_range_fault(struct dmirror *dmirror,
>  		break;
>  	}
>  
> -	ret = dmirror_do_fault(dmirror, range);
> +	/* update device page table after migration */
> +	if (!migrate)
> +		ret = dmirror_do_fault(dmirror, range);
>  
>  	mutex_unlock(&dmirror->mutex);
>  out:

[Severity: High]
Does skipping the page table update here leave us vulnerable to concurrent
unmaps during migration?

When migrate is true, the dmirror->mutex is dropped here without verifying
the final sequence or updating the device page table. This opens a race
window.

[ ... ]
> @@ -1377,6 +1387,114 @@ static int dmirror_migrate_to_device(struct dmirror *dmirror,
>  	return ret;
>  }
>  
> +static int do_fault_and_migrate(struct dmirror *dmirror, struct hmm_range *range)
> +{
> +	struct migrate_vma *migrate = range->migrate;
> +	int ret;
> +
> +	mmap_read_lock(dmirror->notifier.mm);
> +
> +	/* Fault-in pages for migration */
> +	ret = dmirror_range_fault(dmirror, range);
[ ... ]
> +	pr_debug("Migrating from sys mem to device mem\n");
> +
> +	dmirror_migrate_alloc_and_copy(migrate, dmirror);
> +	migrate_vma_pages(migrate);
> +	dmirror_migrate_finalize_and_map(migrate, dmirror);
> +	migrate_vma_finalize(migrate);

[Severity: High]
Can this sequence lead to a use-after-free of device pages if a concurrent
unmap occurs after we dropped the mutex in dmirror_range_fault()?

While mmap_read_lock() is held here, a concurrent madvise(MADV_DONTNEED)
could still execute (since it only requires the read lock or per-VMA lock)
and clear the newly installed migration PTEs, triggering the MMU interval
notifier.

Since the interval notifier invalidation would see an empty dmirror->pt (as
we haven't mapped the pages yet), it would take no action. 

Then, dmirror_migrate_finalize_and_map() inserts the newly allocated device
pages into dmirror->pt blindly, without re-checking
mmu_interval_read_retry(). 

When migrate_vma_finalize() runs, it would fail to map the device page into
the CPU page table because the migration PTE is gone, dropping the final
reference and freeing the page while it is still mapped in the device
page table.

> +out:
> +	mmap_read_unlock(dmirror->notifier.mm);
> +	return ret;
> +}

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