Re: [PATCH v6 08/12] dma-buf: heaps: Add debugfs support
From: sashiko-bot@kernel.org
Date: 2026-09-04 11:34:37
Also in:
dri-devel, linux-s390, linux-trace-kernel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [Medium] Incomplete resource cleanup in the newly added `dma_heap_exit()` leaves dynamically allocated heaps, character devices, and sysfs devices active but orphaned. Pre-existing issues: - [High] Race condition in `dma_heap_add` error path leads to a critical Use-After-Free of `struct dma_heap` due to premature device exposure. -- commit e23fa30f3769a236faddd443565a8ccf6a9af125 Author: Thierry Reding [off-list ref] dma-buf: heaps: Add debugfs support This commit adds a callback to struct dma_heap_ops to show heap state in debugfs. It creates a top-level dma_heap directory where individual heap files are registered. A new dma_heap_exit() function is also introduced to clean up the class and debugfs directory on module exit.
quoted hunk ↗ jump to hunk
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index 8a6c2b6e0cc60..65c80dac65009 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -299,6 +334,13 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info) /* Add heap to the list */ list_add(&heap->list, &heap_list); + +#ifdef CONFIG_DEBUG_FS + if (heap->ops && heap->ops->show) + debugfs_create_file(heap->name, 0444, dma_heap_debugfs_dir, + heap, &dma_heap_debug_fops); +#endif + mutex_unlock(&heap_list_lock); return heap;
[Severity: High]
This isn't a bug introduced by this patch, but is it possible for a concurrent
userspace process to trigger a use-after-free during the error path of
dma_heap_add()?
Earlier in dma_heap_add(), the character device is exposed to userspace before
the name uniqueness check is performed:
ret = cdev_add(&heap->heap_cdev, heap->heap_devt, 1);
...
dev_ret = device_create(dma_heap_class, ...);
...
/* check the name is unique */
list_for_each_entry(h, &heap_list, list) {
if (!strcmp(h->name, exp_info->name)) {
...
goto err3;
}
}
If another driver attempts to register a heap with a duplicate name, could a
userspace process open the device before the uniqueness check fails? This would
store the heap pointer in the open file descriptor. When the uniqueness check
then fails, the error path executes kfree(heap), leaving the open file
descriptor pointing to freed memory.
[ ... ]
quoted hunk ↗ jump to hunk
@@ -335,6 +377,16 @@ static int dma_heap_init(void) } dma_heap_class->devnode = dma_heap_devnode; + dma_heap_init_debugfs(); + return 0; } subsys_initcall(dma_heap_init); + +static void __exit dma_heap_exit(void) +{ + dma_heap_exit_debugfs(); + class_destroy(dma_heap_class);
[Severity: Medium] Does this successfully tear down the resources if there are dynamically allocated heaps still registered? The dma_heap_exit() function destroys the sysfs class by calling class_destroy(), but it doesn't appear to iterate over heap_list to cleanly tear down the individual registered heaps. Can this leave dangling references and lead to sysfs issues, since device_destroy() and cdev_del() are never called for any active heaps before the class is destroyed?
+ unregister_chrdev_region(dma_heap_devt, NUM_HEAP_MINORS); +} +__exitcall(dma_heap_exit);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260904-tegra-vpr-v6-0-79042cfa8de5@nvidia.com?part=8