Re: [PATCH v2] drm/pagemap: no need to recheck own pagemap in migrate_to_devmem

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Migration error paths pass unencoded raw PFNs to migrate_vma_pages(), causing out-of-bounds dereferences or mapping arbitrary physical memory into userspace.
--

--- Patch [1]: [PATCH v2] drm/pagemap: no need to recheck own pagemap in migrate_to_devmem ---
commit f37b1a946d737409d8a543e854c0c96738771e75
Author: Zhaoyu Liu <liuzhaoyu.zackary@picoheart.com>

drm/pagemap: no need to recheck own pagemap in migrate_to_devmem
    
The second loop in drm_pagemap_migrate_to_devmem() checks to skip pages
already on the target pagemap. However, this condition is always false at
this point since the first loop already aborts if necessary. This commit
removes the dead code and simplifies the error jumping paths.

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index 15c78eca180b7..64922b944092c 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c

[ ... ]

> @@ -786,7 +779,6 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
>  			goto err_finalize;
>  		}
>  
> -next:
>  		i += NR_PAGES(order);
>  	}

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential problem in the
error handling path during the migration loop.

If drm_pagemap_migrate_range() fails in the middle of this loop, the local
npages variable is truncated to i + 1 before jumping to err_finalize. 
However, err_finalize falls through to err_aborted_migration:

err_finalize:
	if (err)
		drm_pagemap_migration_unlock_put_pages(npages, migrate.dst);
err_aborted_migration:
	migrate_vma_pages(&migrate);

Because migrate_vma_pages() uses the struct field migrate.npages, it will
process the entire original array length rather than the truncated local
npages value. The remaining elements in migrate.dst still contain unencoded
raw PFNs previously populated by ops->populate_devmem_pfn().

If one of these raw PFNs happens to be odd (bit 0 set), it matches
MIGRATE_PFN_VALID. This could cause the kernel to shift it by 6 and use it
as a struct page pointer, leading to an out-of-bounds dereference or mapping
arbitrary physical memory into userspace.

Could this error path be updated to avoid passing unencoded PFNs to
migrate_vma_pages() when the migration loop aborts early?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/anSKZabjuhl5Ojl4@hostpc?part=1