Re: [PATCH v5] drm/pagemap: Prevent double migration of device pages

[sashiko-bot@kernel.org]

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

New issues:
- [High] Incorrect PFN array iteration when encountering large folios causes unrelated folios to be skipped, and leaves tail pages improperly processed.

Pre-existing issues:
- [High] Out-of-bounds array write in `migrate_device_pfns()` when processing compound pages.
--

--- Patch [1]: [PATCH v5] drm/pagemap: Prevent double migration of device pages ---
commit e82d028d2739c02084b327b5c44a4c4cb949a76c
Author: Arvind Yadav <arvind.yadav@intel.com>
Subject: drm/pagemap: Prevent double migration of device pages

This patch aims to prevent soft lockups caused by double migration of
device-private folios. It marks successfully migrated folios in their
zone_device_data and makes both CPU-fault and raw-PFN migration paths
skip folios carrying this flag.

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
[ ... ]
> +static void drm_pagemap_skip_retired_pages(unsigned long *src_pfns,
> +					   unsigned long npages)
> +{
> +	unsigned long i = 0;
> +
> +	while (i < npages) {
> +		struct page *page = migrate_pfn_to_page(src_pfns[i]);
> +
> +		if (!page) {
> +			i++;
> +			continue;
> +		}
> +
> +		if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
> +		    is_device_private_page(page) &&
> +		    (drm_pagemap_page_get_flags(page) &
> +		     DRM_PAGEMAP_ZDD_FLAG_MIGRATED))
> +			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> +

[Severity: High]
Does advancing by folio_nr_pages() here leave tail pages improperly
processed?

By only clearing the MIGRATE_PFN_MIGRATE bit for src_pfns[i] (the head page)
and then skipping ahead, any subsequent tail pages in the src_pfns array
will remain flagged.

Downstream functions like migrate_vma_pages() process every array entry, so
they might see the un-cleared tail pages and attempt to migrate them
individually. Could this defeat the fix and trigger the double migration
lockups again?

Also, if the PFN array starts on a tail page due to VMA boundary clamping,
will advancing by the full folio_nr_pages() overshoot the folio's actual
footprint in the array and skip completely unrelated folios?

This same pattern appears in drm_pagemap_retire_migrated_pages() just above.
There are also pre-existing loops in drm_pagemap_evict_to_ram(),
__drm_pagemap_migrate_to_ram(), and drm_pagemap_migrate_populate_ram_pfn()
that advance using i += NR_PAGES(order) and might suffer from this same
tail-page stepping issue.

> +		i += folio_nr_pages(page_folio(page));
> +	}
> +}
[ ... ]
> @@ -1143,6 +1240,8 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem *devmem_allocation)
>  	if (err)
>  		goto err_free;
>  
> +	drm_pagemap_skip_retired_pages(src, npages);
> +

[Severity: High]
This is a pre-existing issue, but does migrate_device_pfns() (called just
above this in drm_pagemap_evict_to_ram) lack bounds checking when processing
compound pages?

When the following loop in mm/migrate_device.c:migrate_device_pfns()
encounters a compound page, it zeroes out subsequent entries in the src_pfns
array up to the full size of the compound page:

mm/migrate_device.c:migrate_device_pfns() {
    ...
    for (j = 1; j < nr; j++)
        src_pfns[i+j] = 0;
    ...
}

If a large folio extends beyond the npages limit, such as when the caller
passes a clamped range or an unaligned array size, this loop will write
zeros past the end of the src_pfns allocation.

Could this corrupt adjacent memory, like the uninitialized dst array
allocated concurrently?

>  	err = drm_pagemap_migrate_populate_ram_pfn(NULL, NULL, npages, &mpages,
>  						   src, dst, 0);
>  	if (err || !mpages)
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810070240.2721317-1-arvind.yadav@intel.com?part=1