Re: [PATCH] drm/pagemap: Reset migration page count on eviction retry

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] DMA mappings are leaked in migration error paths because the destination page array is zeroed before unmapping.
- [High] DMA unmapping occurs after pages are exposed to the CPU, violating cache coherency rules.
--

--- Patch [1]: [PATCH] drm/pagemap: Reset migration page count on eviction retry ---
commit f4a68acf50f418cad59d91b95424edfe21778ec5
Author: Arvind Yadav <arvind.yadav@intel.com>

drm/pagemap: Reset migration page count on eviction retry

This commit updates drm_pagemap_evict_to_ram() to reset the mpages
counter to 0 at the retry label. This ensures that retried eviction
attempts do not incorrectly proceed using the page count from the
previous attempt.

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index 15c78eca180b7..695401cb2da24 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
[ ... ]
> @@ -1121,6 +1121,7 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem *devmem_allocation)
>  	npages = devmem_allocation->size >> PAGE_SHIFT;
>  
>  retry:
> +	mpages = 0;
>  	if (!mmget_not_zero(devmem_allocation->mm))
>  		return -EFAULT;
>  

Further down in drm_pagemap_evict_to_ram():

> err_finalize:
> 	if (err)
> 		drm_pagemap_migration_unlock_put_pages(npages, dst);

[Severity: High]
This isn't a bug introduced by this patch, but does this error path leak
DMA mappings?

If an error occurs (such as a failure to copy to RAM),
drm_pagemap_migration_unlock_put_pages() is called, which zeroes the
destination page array entries in dst.

Subsequently, when drm_pagemap_migrate_unmap_pages() iterates over these
entries, it skips unmapping because the page evaluates to NULL:

drm_pagemap_migrate_unmap_pages() {
    ...
    struct page *page = migrate_pfn_to_page(migrate_pfn[i]);

    if (!page || !pagemap_addr[i].addr || dma_mapping_error(dev, pagemap_addr[i].addr))
        goto next;
    ...
}

Could this cause the DMA addresses to remain permanently mapped in the IOMMU
or SWIOTLB?

> 	migrate_device_pages(src, dst, npages);
> 	migrate_device_finalize(src, dst, npages);
> 	drm_pagemap_migrate_unmap_pages(devmem_allocation->dev, pagemap_addr, dst, npages,
> 					DMA_FROM_DEVICE, &state);

[Severity: High]
This is also a pre-existing issue, but is the ordering of CPU exposure and
DMA unmapping safe here?

migrate_device_finalize() is called before drm_pagemap_migrate_unmap_pages().

The finalize function removes migration PTEs and restores normal CPU page
table entries, allowing userspace to immediately access the migrated pages.

However, the DMA API requires that dma_unmap_page() completes before the
CPU accesses the memory. Because dma_unmap_page() for DMA_FROM_DEVICE
mappings invalidates the CPU cache on non-coherent architectures, executing
it after the CPU has been granted access could destroy recent CPU writes or
cause the CPU to read stale speculative cache lines.

The identical unsafe ordering appears to be present in
__drm_pagemap_migrate_to_ram() where migrate_vma_finalize() is called.

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