Claude review: misc: fastrpc: Allocate entire reserved memory for Audio PD in probe

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

Patch Review

**Verdict: Needs fixes — two memory leaks in probe error paths, and a logic issue with `audio_init_mem` reset.**

**Bug 1: Memory leak on kzalloc failure in probe**

```c
+			if (domain_id == ADSP_DOMAIN_ID) {
+				data->remote_heap =
+					kzalloc_obj(*data->remote_heap, GFP_KERNEL);
+				if (!data->remote_heap)
+					return -ENOMEM;
```

At this point, `data` has already been allocated (line ~2373). The bare `return -ENOMEM` leaks `data`. This should be `goto err_free_data` or equivalent. Also, note that `kzalloc_obj(*data->remote_heap)` without explicit `GFP_KERNEL` would be equivalent since `default_gfp()` defaults to `GFP_KERNEL`.

**Bug 2: Memory leak on SCM failure in probe**

If `qcom_scm_assign_mem()` fails after `data->remote_heap` was allocated:

```c
			err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
				    data->vmperms, data->vmcount);
			if (err)
				goto err_free_data;
```

The `err_free_data` label only does `kfree(data)` — it does not free `data->remote_heap`. This leaks the remote_heap allocation.

**Issue 3: Unconditional `audio_init_mem = false` in err_invoke**

```c
 err_invoke:
-	if (fl->cctx->vmcount && scm_done) { ... }
-err_map:
-	fastrpc_buf_free(fl->cctx->remote_heap);
-	fl->cctx->remote_heap = NULL;
-err_name:
+	fl->cctx->audio_init_mem = false;
```

If two concurrent callers enter `fastrpc_init_create_static_process()`, the first one sets `audio_init_mem = true` and succeeds. The second caller takes the `else` branch (pageslen=0, pages=0). If the second caller's invoke fails, `audio_init_mem` is unconditionally reset to `false`. A subsequent third caller would then re-send the memory information to the DSP, which the DSP may have already received from the first call. The reset should be conditional:

```c
err_invoke:
	if (inbuf.pageslen)
		fl->cctx->audio_init_mem = false;
```

**Style nit: Inconsistent cctx usage**

The patch adds `struct fastrpc_channel_ctx *cctx = fl->cctx;` but then mixes `fl->cctx->` and `cctx->` in the same code block:

```c
+	spin_lock_irqsave(&cctx->lock, flags);
+	if (!fl->cctx->audio_init_mem) {
+		if (!fl->cctx->remote_heap ||
```

Should consistently use `cctx->` throughout.

**Observation on remove path**

The remove path correctly uses `kfree()` instead of `fastrpc_buf_free()` since the buffer was allocated with `kzalloc_obj` (not DMA coherent allocation). However, if `qcom_scm_assign_mem` fails during remove, the remote_heap struct is never freed:

```c
+		err = qcom_scm_assign_mem(cctx->remote_heap->dma_addr,
+					  cctx->remote_heap->size, &src_perms,
+					  &dst_perms, 1);
+		if (!err)
+			kfree(cctx->remote_heap);
```

If the SCM call fails during teardown, there's not much that can be done, but a `dev_err()` log would be helpful for debugging, and the memory should arguably be freed anyway since the channel is shutting down (the SCM failure means the memory ownership is in a bad state, but leaking the tracking struct doesn't help).

Also: if `cctx->vmcount` is 0 but `cctx->remote_heap` is non-NULL, the entire block is skipped and `remote_heap` is leaked. This could happen if the ADSP domain has reserved-memory in DT but no `qcom,vmids`.

---

---
Generated by Claude Code Patch Reviewer