Re: [PATCH] drm/pagemap: Clear driver-provided PFNs from migration PFN array

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] If `ops->populate_devmem_pfn()` fails, `migrate.dst` is not cleared of unencoded driver PFNs, leading to wild pointer dereferences and memory corruption in `migrate_vma_pages()`.
--

--- Patch [1]: [PATCH] drm/pagemap: Clear driver-provided PFNs from migration PFN array ---
commit ed7304923def6f0807fcb47377684c7a1ab58012
Author: Matthew Brost <matthew.brost@intel.com>

drm/pagemap: Clear driver-provided PFNs from migration PFN array

This patch clears remaining driver-provided PFNs from the migration PFN
array when an error occurs during the incremental copy phase. This prevents
migrate_vma_*() functions from processing unencoded device PFNs.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 3902846af36b ("drm/pagemap Fix error paths in drm_pagemap_migrate_to_devmem")
Fixes: ec265e1f1cfc ("drm/pagemap: Support source migration over interconnect")
Cc: stable@vger.kernel.org
Signed-off-by: Matthew Brost <matthew.brost@intel.com>

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index 15c78eca180b7..f991b296c8e19 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
> @@ -807,8 +807,11 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
>  		msecs_to_jiffies(mdetails->timeslice_ms);
>  
>  err_finalize:
> -	if (err)
> +	if (err) {
>  		drm_pagemap_migration_unlock_put_pages(npages, migrate.dst);
> +		for (i = npages; i < npages_in_range(start, end); ++i)
> +			migrate.dst[i] = 0;
> +	}
>  err_aborted_migration:
>  	migrate_vma_pages(&migrate);

[Severity: High]
This is a pre-existing issue, but I noticed an error path that might still
bypass this new zeroing loop.

Earlier in drm_pagemap_migrate_to_devmem(), if ops->populate_devmem_pfn()
fails, the code jumps directly to err_aborted_migration:

	err = ops->populate_devmem_pfn(devmem_allocation, npages, migrate.dst);
	if (err)
		goto err_aborted_migration;

Since this jumps directly to err_aborted_migration, it skips the err_finalize
block completely. If migrate.dst was partially populated with unencoded driver
PFNs before the failure, wouldn't migrate_vma_pages() then process those
raw PFNs?

If a raw driver-provided PFN happens to have its lowest bit set,
migrate_pfn_to_page() could interpret it as MIGRATE_PFN_VALID and return a
bogus struct page pointer, which might lead to memory corruption.

Should the populate_devmem_pfn() failure jump to err_finalize instead, or
should the array clearing be moved to cover all abort paths?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721201959.1082266-1-matthew.brost@intel.com?part=1