Re: [PATCH v2 2/5] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds write in `migrate_device_range()` and `migrate_device_pfns()` when processing compound device private folios.
--

--- Patch [2]: [PATCH v2 2/5] mm/migrate_device: Fix THP splitting of a CPU faulted device private folio ---
commit 66cc9eb01f0258173048c3a225ebb9ca65d56410
Author: Matthew Brost <[email protected]>

mm/migrate_device: Fix THP splitting of a CPU faulted device private folio

This commit fixes two issues when splitting a device private THP during a CPU
fault: it drops and re-acquires the fault reference across the split to prevent
it from failing, and walks the rmap to demote all PMD sized migration entries 
before splitting to prevent leaving stale huge PMDs in other VMAs.

[ ... ]

> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 1e24039f62b33..155a182c528f8 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -1187,8 +1268,7 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>  					goto next;
>  				}
>  				nr = 1 << folio_order(folio);
> -				addr = migrate->start + i * PAGE_SIZE;
> -				if (migrate_vma_split_unmapped_folio(migrate, i, addr, folio)) {
> +				if (migrate_vma_split_unmapped_folio(migrate, i, folio)) {
>  					src_pfns[i] &= ~(MIGRATE_PFN_MIGRATE |
>  							 MIGRATE_PFN_COMPOUND);
>  					goto next;

[Severity: High]
This is a pre-existing issue, but does migrate_device_range() write out of
bounds when processing a compound device private folio?

Looking at mm/migrate_device.c:migrate_device_range(), the code iterates
through pfns:

	for (pfn = start, i = 0; i < npages; pfn++, i++) {
		...
		nr = folio_nr_pages(folio);
		if (nr > 1) {
			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
			for (j = 1; j < nr; j++)
				src_pfns[i+j] = 0;
			i += j - 1;
			pfn += j - 1;
		}
	}

If npages does not fully cover the compound folio, this loop writes zeroes
into src_pfns without checking if i + j is less than npages. Because
src_pfns is allocated by the caller to exactly npages in size, writing up
to nr - 1 zeroes can overflow the array.

The exact same missing bounds check appears in migrate_device_pfns():

	for (i = 0; i < npages; i++) {
		...
		nr = folio_nr_pages(folio);
		if (nr > 1) {
			src_pfns[i] |= MIGRATE_PFN_COMPOUND;
			for (j = 1; j < nr; j++)
				src_pfns[i+j] = 0;
			i += j - 1;
		}
	}

Could these loops be updated to check i + j < npages before writing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2