Re: [PATCH] drm/pagemap: Guard HPAGE_PMD_ORDER use with CONFIG_ARCH_ENABLE_THP_MIGRATION

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Aborting a migration mid-loop leaves raw device PFNs in the migration array, causing a bogus pointer dereference and kernel panic in migrate_vma_pages().
--

--- Patch [1]: [PATCH] drm/pagemap: Guard HPAGE_PMD_ORDER use with CONFIG_ARCH_ENABLE_THP_MIGRATION ---
commit 66a306e51148532e3b7d883d3232632355c05797
Author: Jan Stancek <jstancek@redhat.com>

drm/pagemap: Guard HPAGE_PMD_ORDER use with CONFIG_ARCH_ENABLE_THP_MIGRATION

This commit fixes a compile error when both CONFIG_TRANSPARENT_HUGEPAGE
and CONFIG_HUGETLB_PAGE are disabled. It guards HPAGE_PMD_ORDER uses with
CONFIG_ARCH_ENABLE_THP_MIGRATION.

Link: https://lore.kernel.org/r/202607201914.LpAGsbXs-lkp@intel.com/

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index 15c78eca180b7..53a78752c8be3 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
[ ... ]
> @@ -763,7 +765,10 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
>  		}
>  		migrate.dst[i] = migrate_pfn(migrate.dst[i]);
>  
> +#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
>  		if (migrate.src[i] & MIGRATE_PFN_COMPOUND) {
> +			unsigned long j;
> +
>  			drm_WARN_ONCE(dpagemap->drm, src_page &&
>  				      folio_order(page_folio(src_page)) != HPAGE_PMD_ORDER,
>  				      "Unexpected folio order\n");
> @@ -774,6 +779,7 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
>  			for (j = 1; j < NR_PAGES(order) && i + j < npages; j++)
>  				migrate.dst[i + j] = 0;
>  		}
> +#endif
>  
>  		drm_pagemap_get_devmem_page(page, order, zdd);

[Severity: Critical]
This is a pre-existing issue, but can the error path here leave raw device
PFNs in the migration array, causing a bogus pointer dereference in
migrate_vma_pages()?

In drm_pagemap_migrate_to_devmem(), ops->populate_devmem_pfn() populates
migrate.dst with sequential raw device PFNs. The loop processes these and
converts them into the migrate_pfn() format. If a migration batch fails
mid-loop:

		err = drm_pagemap_migrate_range(...);
		if (err) {
			npages = i + 1;
			goto err_finalize;
		}

The error path zeros out migrate.dst entries up to index i, and then
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() iterates up to the original migrate.npages,
unprocessed entries (where index > i) still contain unformatted raw PFNs.
Since PFNs are sequential, half of them are odd numbers, meaning bit 0
(MIGRATE_PFN_VALID) is set.

Will __migrate_device_pages() misinterpret these odd raw PFNs as valid
migration entries, leading to a dereference of a completely bogus
struct page pointer when it calls pfn_to_page()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/84a35e16f72330d6e7b2d6e969040f36959e2dad.1784630290.git.jstancek@redhat.com?part=1