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 <[email protected]>

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/[email protected]/

> 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.jstan...@redhat.com?part=1

Reply via email to