Claude review: dma-buf: dma-heap: split dma_heap_add

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

This is the core refactoring, splitting `dma_heap_add()` into `dma_heap_create()` / `dma_heap_register()` / `dma_heap_destroy()`.

**Issue: `heap->name` is not duplicated — double-free risk**

In `dma_heap_create()`:
```c
	heap->name = exp_info->name;
```

The name is stored as a raw pointer from `exp_info`. But `dma_heap_dev_release()` does:
```c
	kfree(heap->name);
```

If the caller passed a string literal or stack-allocated name (which `dma_heap_add()` callers like system_heap and cma_heap do with string literals), this `kfree()` will corrupt memory. Looking at the coherent heap (patch 5), `coh_heap->name = kstrdup(rmem->name, GFP_KERNEL)` is passed in, so it's safe for that caller. But *any existing caller* of `dma_heap_add()` will now have their name kfree'd in the release path. The original code (pre-series) never freed `heap->name` — the heap struct was freed directly in the error path, and heaps were never removed. This is a **regression for existing callers**.

The `dma_heap_create()` should `kstrdup()` the name, or `dma_heap_dev_release()` should not free it.

**Issue: `dma_heap_register()` error path calls `dma_heap_destroy()` — double destroy possible**

In `dma_heap_register()` error paths, `err0` calls `dma_heap_destroy(heap)`:
```c
err0:
	dma_heap_destroy(heap);
	return ret;
```

And `dma_heap_add()` on register failure also calls:
```c
	ret = dma_heap_register(heap);
	if (ret) {
		pr_err("dma_heap: failed to register heap (%d)\n", ret);
		dma_heap_destroy(heap);
		return ERR_PTR(ret);
	}
```

So `dma_heap_destroy()` (which is `put_device()`) is called **twice** — once inside `dma_heap_register()` and once in `dma_heap_add()`. After `device_initialize()`, the refcount is 1. The first `put_device()` drops it to 0 and triggers the release callback, freeing the heap. The second `put_device()` is a UAF. This is a **bug**.

**Issue: `err3` path does `device_destroy()` then falls through to `dma_heap_destroy()`**

```c
err3:
	device_destroy(dma_heap_class, heap->heap_devt);
err2:
	cdev_del(&heap->heap_cdev);
err1:
	xa_erase(&dma_heap_minors, minor);
err0:
	dma_heap_destroy(heap);
```

After `device_add()` succeeds, `err3` calls `device_destroy()` which does `device_unregister()` → `device_del()` + `put_device()`. Then execution falls through to `err0` which calls `dma_heap_destroy()` → `put_device()` again. Since `device_initialize()` sets refcount to 1 and `device_add()` doesn't bump it, the `device_destroy()`'s `put_device()` drops to 0 and frees, then `dma_heap_destroy()` is UAF. This needs restructuring.

**Minor: `kzalloc_obj` usage**

```c
	heap->heap_dev = kzalloc_obj(*heap->heap_dev);
```

This appears to be a custom macro. It works but is unconventional — `kzalloc(sizeof(*heap->heap_dev), GFP_KERNEL)` is the standard pattern.

---

---
Generated by Claude Code Patch Reviewer