Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] A race condition in `dma_heap_add()` allows userspace to trigger an exploitable use-after-free by opening the character device before the initialization completes. -- --- Patch [6]: [PATCH v5 06/10] dma-buf: heaps: Add debugfs support --- commit f236972ccf6e5f7088156d7adcf0aed10121c6c6 Author: Thierry Reding <[email protected]> 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. > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c > index 3937dd41bb0f6..f279b8fdd3ce0 100644 > --- a/drivers/dma-buf/dma-heap.c > +++ b/drivers/dma-buf/dma-heap.c [ ... ] > @@ -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 > + [Severity: High] This is a pre-existing issue, but is there a race condition in this function that could lead to a use-after-free? In dma_heap_add(), cdev_add() is called earlier, making the device accessible. If the duplicate name check fails, it goes to the error path, destroying the device and freeing the heap structure: drivers/dma-buf/dma-heap.c:dma_heap_add() { ... ret = cdev_add(&heap->heap_cdev, heap->heap_devt, 1); ... list_for_each_entry(h, &heap_list, list) { if (!strcmp(h->name, exp_info->name)) { ... goto err3; } } ... err3: device_destroy(dma_heap_class, heap->heap_devt); ... err0: kfree(heap); return err_ret; } If a user concurrently opens the device between cdev_add() and the kfree() on the error path, would the resulting file descriptor retain a dangling pointer to the freed heap structure in file->private_data? Could a subsequent ioctl() on this file descriptor then dereference the freed heap, triggering a use-after-free? > mutex_unlock(&heap_list_lock); > > return heap; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
