Re: [PATCH v3 6/6] drm/pagemap: Add fault injection for higher-order RAM folio allocation

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-After-Free in debugfs due to missing cleanup on module unload.
--

--- Patch [6]: [PATCH v3 6/6] drm/pagemap: Add fault injection for higher-order RAM folio allocation ---
commit fdb5d198639794ed363bbe0eda839a0e39a16e0c
Author: Matthew Brost <[email protected]>

drm/pagemap: Add fault injection for higher-order RAM folio allocation

This commit introduces a fault injection attribute to simulate memory
pressure for higher-order RAM folio allocations. It adds a debugfs entry
to allow testing the order-0 fallback and THP split paths in
__migrate_device_pages().

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c

[ ... ]

> @@ -1554,6 +1578,16 @@ void drm_pagemap_destroy(struct drm_pagemap *dpagemap, bool is_atomic_or_reclaim
>  		kfree(dpagemap);
>  }
>  
> +static int __init drm_pagemap_module_init(void)
> +{
> +#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_FAULT_INJECTION)
> +	fault_create_debugfs_attr("drm_pagemap_fault_inject", NULL,
> +				  &migrate_to_ram_fault_inject);
> +#endif
> +	return 0;
> +}
> +module_init(drm_pagemap_module_init);
> +
>  static void drm_pagemap_exit(void)
>  {
>  	flush_work(&drm_pagemap_work);
>  	if (WARN_ON(!llist_empty(&drm_pagemap_unhold_list)))
>  		disable_work_sync(&drm_pagemap_work);
>  }

[Severity: High]
Does this leak the debugfs directory on module unload, potentially leading
to a use-after-free?

In drm_pagemap_module_init(), fault_create_debugfs_attr() creates a debugfs
entry pointing to migrate_to_ram_fault_inject, which resides in the module's
memory. Since the returned dentry isn't saved and removed here in
drm_pagemap_exit(), the debugfs entry will remain active after the module is
unloaded.

If the debugfs files are accessed post-unload, could the kernel dereference
the freed fault_attr pointer? Should the dentry be captured during init and
cleaned up in the exit function?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6