Thread (58 messages) flat view 58 messages, 10 authors, 2026-08-10

Re: [PATCH v3 07/11] dma-buf: heaps: Add debugfs support

From: sashiko-bot@kernel.org
Date: 2026-07-01 16:27:29
Also in: dri-devel, linux-s390

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] debugfs_create_dir failure handling causes namespace pollution.
- [Low] Incomplete resource teardown in newly added dma_heap_exit() function.

Pre-existing issues:
- [High] Use-After-Free due to character device exposure before uniqueness validation completes.
--

commit be3535d09b01d53c055d9d5fb7c60f03232a5ac7
Author: Thierry Reding [off-list ref]

dma-buf: heaps: Add debugfs support

Add a callback to struct dma_heap_ops that heap providers can implement
to show information about the state of the heap in debugfs. A top-level
directory named "dma_heap" is created in debugfs and individual files
will be named after the heaps.
quoted hunk ↗ jump to hunk
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index a76bf3f8b071a..1ceb6ee8c05a2 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -224,6 +225,46 @@ const char *dma_heap_get_name(struct dma_heap *heap)
[ ... ]
+static struct dentry *dma_heap_debugfs_dir;
+
+static void dma_heap_init_debugfs(void)
+{
+	struct dentry *dir;
+
+	dir = debugfs_create_dir("dma_heap", NULL);
+	if (IS_ERR(dir))
+		return;
+
+	dma_heap_debugfs_dir = dir;
+}
[Severity: Low]
If debugfs_create_dir() returns an error, this early return leaves
dma_heap_debugfs_dir as NULL.

Later, when dma_heap_add() calls debugfs_create_file() with this NULL parent,
won't it create the heap files directly in the debugfs root directory instead
of failing safely?

[ ... ]
quoted hunk ↗ jump to hunk
@@ -298,6 +339,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 is a pre-existing issue, but doesn't the current design of
dma_heap_add() allow a use-after-free?

The character device is made live and exposed to userspace early:

	cdev_init(&heap->heap_cdev, &dma_heap_fops);
	ret = cdev_add(&heap->heap_cdev, heap->heap_devt, 1);

But then, if the uniqueness check fails:

	list_for_each_entry(h, &heap_list, list) {
		if (!strcmp(h->name, exp_info->name)) {
			...
			goto err3;
		}

The error path frees the heap without revoking the live character device:

err0:
	kfree(heap);

If userspace opens the device concurrently before the uniqueness check fails,
could it access the freed heap object?

[ ... ]
quoted hunk ↗ jump to hunk
@@ -334,6 +382,14 @@ 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();
+}
+__exitcall(dma_heap_exit);
[Severity: Low]
Does this exit function omit teardown steps?

It cleans up debugfs but seems to be missing calls to
unregister_chrdev_region() and class_destroy() that were set up in
dma_heap_init().

If this subsystem is ever built as a loadable module, won't this leak those
resources on unload?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-tegra-vpr-v3-0-d80f7b871bb4@nvidia.com?part=7
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help