Re: [PATCH 4/4] 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] The debugfs entry created for fault injection is not removed on module unload, leading to a memory leak and a use-after-free vulnerability.
--

--- Patch [4]: [PATCH 4/4] drm/pagemap: Add fault injection for higher-order RAM folio allocation ---
commit 20731b2cfca08a84398b3c52bd2ca16474a3a4f7
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 the failure
of higher-order allocations in drm_pagemap_migrate_populate_ram_pfn().
The feature is exposed via a debugfs entry.

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index 7610e233d238c..01eb29ef023ce 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c

[ ... ]

> @@ -1540,6 +1564,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);

[Severity: High]
Does this code leave dangling debugfs entries on module unload?

In drm_pagemap_module_init(), fault_create_debugfs_attr() creates the
drm_pagemap_fault_inject debugfs directory, which holds pointers to the
migrate_to_ram_fault_inject struct located in the module's .data section.

However, drm_pagemap_exit() doesn't seem to clean this up by tracking the
dentry and calling debugfs_remove_recursive(). When the module is
unloaded, the memory backing the fault attribute is freed, but the debugfs
files remain.

Could reading or writing to these lingering debugfs files after the
module is unloaded trigger a use-after-free?

>  	if (WARN_ON(!llist_empty(&drm_pagemap_unhold_list)))
>  		disable_work_sync(&drm_pagemap_work);
>  }

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