Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Missing debugfs cleanup on module unload causes a use-after-free vulnerability. Pre-existing issues: - [High] DMA mappings are leaked on the error path because the destination PFN array is zeroed before it can be unmapped. -- --- Patch [5]: [PATCH v2 5/5] drm/pagemap: Add fault injection for higher-order RAM folio allocation --- commit 8131ac1d5ae4485b09ae7fb3c943731bd418c771 Author: Matthew Brost <[email protected]> Subject: drm/pagemap: Add fault injection for higher-order RAM folio allocation This commit adds a fault injection mechanism to test the order-0 fallback and THP split paths in drm_pagemap_migrate_populate_ram_pfn. It achieves this by exposing a debugfs attribute that can artificially force higher-order folio allocations to fail. > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c > index 05eb7254028f7..01c639da767c3 100644 > --- 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); [Severity: High] Does the module unload path need to clean up this debugfs attribute? Because the dentry from fault_create_debugfs_attr is not saved and removed in drm_pagemap_exit, the debugfs files are left behind when the module is unloaded. If userspace accesses these leftover files after unloading, won't this cause a use-after-free when it tries to read or write the freed migrate_to_ram_fault_inject struct? [Severity: High] This is a pre-existing issue, but can the error path in __drm_pagemap_migrate_to_ram leak DMA mappings? When an error occurs during migration (for example, in ops->copy_to_ram), the err_finalize label executes the following: __drm_pagemap_migrate_to_ram() { ... err_finalize: drm_pagemap_migration_unlock_put_pages(npages, migrate.dst); drm_pagemap_migrate_unmap_pages(dev, npages, pagemap_addr, migrate.dst); ... } The call to drm_pagemap_migration_unlock_put_pages unlocks the folios and zeroes out the entries in the migrate.dst array. Immediately after, drm_pagemap_migrate_unmap_pages is called to unmap the DMA addresses. However, when the IOVA fast-path is not used, the unmap loop relies entirely on that same array to resolve the page: drm_pagemap_migrate_unmap_pages() { ... for (i = 0; i < npages; i++) { struct page *page = migrate_pfn_to_page(migrate_pfn[i]); if (!page || !pagemap_addr[i].addr || dma_mapping_error(...)) goto next; ... dma_unmap_page(dev, pagemap_addr[i].addr, ...); ... } Since the migrate.dst array was already zeroed by the previous function, the page resolves to NULL. Could this cause the loop to silently take the goto next path and permanently leak the DMA mappings? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
