Claude review: misc: fastrpc: Add reference counting for fastrpc_user structure

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

Patch Review

**kref_init placement is after the user becomes visible (BUG)**

In `fastrpc_device_open()`, the patch places `kref_init()` **after** `list_add_tail()`:

```c
 	spin_lock_irqsave(&cctx->lock, flags);
 	list_add_tail(&fl->user, &cctx->users);
 	spin_unlock_irqrestore(&cctx->lock, flags);
+	kref_init(&fl->refcount);
```

At this point `fl` is already visible on `cctx->users`. If `fastrpc_rpmsg_remove()` runs on another CPU between `list_add_tail()` and `kref_init()`, it will iterate `cctx->users` and could operate on this user with a zero refcount (from `kzalloc` zero-init). The `kref_init()` should be placed alongside the other initialization calls (near `spin_lock_init`, `mutex_init`, etc.) **before** the user is linked into any list:

```c
	fl->is_secure_dev = fdevice->secure;
+	kref_init(&fl->refcount);

	fl->sctx = fastrpc_session_alloc(fl);
	...
	list_add_tail(&fl->user, &cctx->users);
```

**fastrpc_user_free only does partial cleanup — by design, but deserves a comment**

```c
+static void fastrpc_user_free(struct kref *ref)
+{
+	struct fastrpc_user *fl = container_of(ref, struct fastrpc_user, refcount);
+
+	fastrpc_channel_ctx_put(fl->cctx);
+	mutex_destroy(&fl->mutex);
+	kfree(fl);
+}
```

This only frees the struct, the mutex, and the cctx reference. All heavyweight cleanup (session free, DSP process release, pending context teardown, map/mmap cleanup) remains in `fastrpc_device_release()`. This is correct — the refcount only extends the *lifetime of the struct* past device release, not the logical ownership. But a brief comment in `fastrpc_user_free()` noting this would help future readers understand that this is intentional and that full teardown is done in `fastrpc_device_release()`.

**Channel context reference accounting is correct**

I verified there is no double-put of `cctx`. The references are:

| Get | Put |
|-----|-----|
| `fastrpc_device_open()` → `fastrpc_channel_ctx_get(cctx)` | `fastrpc_user_free()` → `fastrpc_channel_ctx_put(fl->cctx)` |
| `fastrpc_context_alloc()` → `fastrpc_channel_ctx_get(cctx)` | `fastrpc_context_free()` → `fastrpc_channel_ctx_put(cctx)` (existing, line 524) |

In `fastrpc_context_free()`, the new `fastrpc_user_put(fl)` is called before the existing `fastrpc_channel_ctx_put(cctx)`:

```c
+	/* Release the reference taken in fastrpc_context_alloc() */
+	fastrpc_user_put(fl);
 	fastrpc_channel_ctx_put(cctx);
```

Even if `fastrpc_user_put()` triggers `fastrpc_user_free()` (which puts the *user's* cctx ref), the *context's* cctx ref still keeps `cctx` alive for the next line. This ordering is safe.

**Error path in fastrpc_context_alloc is correct**

```c
+	fastrpc_user_put(user);
 	fastrpc_channel_ctx_put(cctx);
```

The error path correctly balances the `fastrpc_user_get(user)` taken earlier in the function.

**fastrpc_buf_free safety during workqueue-deferred context_free**

The key UAF scenario the patch addresses: `fastrpc_buf_free()` at `fastrpc.c:417` dereferences `buf->fl->cctx`. In the patched code, when `fastrpc_context_free()` is called from the workqueue, `fastrpc_buf_free(ctx->buf)` runs *before* `fastrpc_user_put(fl)`, so the user struct is guaranteed alive. This is correct.

**Minor: redundant comment on struct member**

```c
+	/* Reference count */
+	struct kref refcount;
```

The `kref` type already implies reference counting. This comment adds no information. Consider dropping it to match the style of `struct kref refcount` used elsewhere in the same file (e.g., in `fastrpc_channel_ctx`).

**Summary of required changes:**

1. **Must fix:** Move `kref_init(&fl->refcount)` before `list_add_tail()` in `fastrpc_device_open()` to close the race window.
2. **Nice to have:** Add a comment in `fastrpc_user_free()` clarifying that full teardown is in `fastrpc_device_release()`, and this only extends struct lifetime.

---
Generated by Claude Code Patch Reviewer