On 8/6/26 9:10 AM, Matthew Brost wrote:
> migrate_device_range() and migrate_device_pfns() zero the tail entries
> of a large folio without checking them against @npages:
>
> for (j = 1; j < nr; j++)
> src_pfns[i+j] = 0;
>
> @nr comes from the folio, not from the array, so a folio that extends
> past the end of the range being migrated writes beyond src_pfns[].
> Callers size that array for @npages entries, so this corrupts whatever
> follows it.
>
> Bound the loop by @npages. The subsequent "i += j - 1" still terminates
> the outer loop correctly: on a bounded exit j is @npages - i, leaving i
> at @npages after the increment.
>
> Reported-by: Sashiko <[email protected]>
> Fixes: a30b48bf1b24 ("mm/migrate_device: implement THP migration of zone
> device pages")
> Cc: Andrew Morton <[email protected]>
> Cc: David Hildenbrand <[email protected]>
> Cc: Lorenzo Stoakes <[email protected]>
> Cc: Zi Yan <[email protected]>
> Cc: Baolin Wang <[email protected]>
> Cc: Liam R. Howlett <[email protected]>
> Cc: Nico Pache <[email protected]>
> Cc: Ryan Roberts <[email protected]>
> Cc: Dev Jain <[email protected]>
> Cc: Barry Song <[email protected]>
> Cc: Lance Yang <[email protected]>
> Cc: Usama Arif <[email protected]>
> Cc: Joshua Hahn <[email protected]>
> Cc: Rakie Kim <[email protected]>
> Cc: Byungchul Park <[email protected]>
> Cc: Gregory Price <[email protected]>
> Cc: Ying Huang <[email protected]>
> Cc: Alistair Popple <[email protected]>
> Cc: Balbir Singh <[email protected]>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Thomas Zimmermann <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Simona Vetter <[email protected]>
> Cc: Thomas Hellström <[email protected]>
> Cc: Francois Dugast <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Assisted-by: GitHub_Copilot:claude-opus-5
> Signed-off-by: Matthew Brost <[email protected]>
> ---
> mm/migrate_device.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 162d29b2807a..ae9027421b80 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -1415,7 +1415,7 @@ int migrate_device_range(unsigned long *src_pfns,
> unsigned long start,
> nr = folio_nr_pages(folio);
> if (nr > 1) {
> src_pfns[i] |= MIGRATE_PFN_COMPOUND;
> - for (j = 1; j < nr; j++)
> + for (j = 1; j < nr && (i + j) < npages; j++)
> src_pfns[i+j] = 0;
I have a similar patch lined up in my clean ups (that I am yet to send out), but
the patch was more along the lines of
nr = folio_nr_pages(folio);
+ if (nr > npages - i) {
+ migrate_device_folio_unlock(folio);
+ src_pfns[i] = 0;
+ continue;
+ }
+
This prevents partial selection, migrate_device_unmap() does take npages as an
argument. We could change the increment of i here to skip past the entire folio.
I am OK with this change as well
> i += j - 1;
> pfn += j - 1;
> @@ -1449,7 +1449,7 @@ int migrate_device_pfns(unsigned long *src_pfns,
> unsigned long npages)
> nr = folio_nr_pages(folio);
> if (nr > 1) {
> src_pfns[i] |= MIGRATE_PFN_COMPOUND;
> - for (j = 1; j < nr; j++)
> + for (j = 1; j < nr && (i + j) < npages; j++)
> src_pfns[i+j] = 0;
> i += j - 1;
> }
Reviewed-by: Balbir Singh <[email protected]>
