[PATCH v2] dma-buf: fix racing conflict of dma_heap_add()

From: Dawei Li
Date: Mon Oct 31 2022 - 10:53:10 EST


Racing conflict could be:
task A task B
list_for_each_entry
strcmp(h->name))
list_for_each_entry
strcmp(h->name)
kzalloc kzalloc
...... .....
device_create device_create
list_add
list_add

The root cause is that task B has no idea about the fact someone
else(A) has inserted heap with same name when it calls list_add,
so a potential collision occurs.

v1: https://lore.kernel.org/all/TYCP286MB2323950197F60FC3473123B7CA349@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/

v1->v2: Narrow down locking scope, check the existence of heap before
insertion, as suggested by Andrew Davis.

Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")

base-commit: 447fb14bf07905b880c9ed1ea92c53d6dd0649d7

Signed-off-by: Dawei Li <set_pte_at@xxxxxxxxxxx>
---
drivers/dma-buf/dma-heap.c | 37 ++++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 8f5848aa144f..1c787a147e3a 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -216,9 +216,21 @@ const char *dma_heap_get_name(struct dma_heap *heap)
return heap->name;
}

+static inline bool dma_heap_exist(const char *name)
+{
+ struct dma_heap *h;
+
+ list_for_each_entry(h, &heap_list, list) {
+ if (!strcmp(h->name, name))
+ return true;
+ }
+
+ return false;
+}
+
struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
{
- struct dma_heap *heap, *h, *err_ret;
+ struct dma_heap *heap, *err_ret;
struct device *dev_ret;
unsigned int minor;
int ret;
@@ -235,13 +247,11 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)

/* check the name is unique */
mutex_lock(&heap_list_lock);
- list_for_each_entry(h, &heap_list, list) {
- if (!strcmp(h->name, exp_info->name)) {
- mutex_unlock(&heap_list_lock);
- pr_err("dma_heap: Already registered heap named %s\n",
- exp_info->name);
- return ERR_PTR(-EINVAL);
- }
+ if (dma_heap_exist(exp_info->name)) {
+ mutex_unlock(&heap_list_lock);
+ pr_err("dma_heap: Already registered heap named %s\n",
+ exp_info->name);
+ return ERR_PTR(-EINVAL);
}
mutex_unlock(&heap_list_lock);

@@ -283,13 +293,22 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
err_ret = ERR_CAST(dev_ret);
goto err2;
}
+
/* Add heap to the list */
mutex_lock(&heap_list_lock);
+ if (unlikely(dma_heap_exist(exp_info->name))) {
+ mutex_unlock(&heap_list_lock);
+ pr_err("dma_heap: Already registered heap named %s\n",
+ exp_info->name);
+ err_ret = ERR_PTR(-EINVAL);
+ goto err3;
+ }
list_add(&heap->list, &heap_list);
mutex_unlock(&heap_list_lock);

return heap;
-
+err3:
+ device_destroy(dma_heap_class, heap->heap_devt);
err2:
cdev_del(&heap->heap_cdev);
err1:
--
2.25.1