Claude review: drm/v3d: fix to avoid cleaning up uninitialized CPU jobs

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

Patch Review

**Bug analysis is correct.** The existing code at lines 1300–1305 (in the patch base) checks `cpu_job->job_type` before `v3d_job_init()` has been called. When no CPU extension is supplied, `job_type` is 0 (from `kcalloc`), the check fails, and the code falls through to `fail:` which calls `v3d_job_cleanup()` → `drm_sched_job_cleanup()` on a job whose `drm_sched_job` base was never initialized. Good catch.

**Issue 1 (build failure): `v3d_cpu_job_free` has the wrong signature and is inaccessible.**

The patch changes the free callback passed to `v3d_job_init`:

```c
ret = v3d_job_init(v3d, file_priv, &cpu_job->base,
		   v3d_cpu_job_free, 0, &se, V3D_CPU);
```

But `v3d_job_init` expects `void (*free)(struct kref *ref)` (see `v3d_submit.c:167`), while the only `v3d_cpu_job_free` in the tree is in `v3d_sched.c:129` with signature `void v3d_cpu_job_free(struct drm_sched_job *sched_job)`. These are incompatible types. Additionally, that function is declared `static`, so it is not visible from `v3d_submit.c`. This will fail to compile.

The original code correctly uses `v3d_job_free` here — the kref-based free callback. If the intent is to have the kref free callback also release the query arrays, a new `v3d_cpu_job_kref_free` function with the correct `struct kref *ref` signature would need to be added in `v3d_submit.c`.

**Issue 2 (memory leak): `fail_init` path leaks query arrays.**

When `v3d_get_extensions()` succeeds and populates `cpu_job->timestamp_query.queries` / `cpu_job->performance_query.queries`, but then `v3d_job_init()` fails:

```c
ret = v3d_job_init(v3d, file_priv, &cpu_job->base,
		   v3d_cpu_job_free, 0, &se, V3D_CPU);
if (ret) {
	v3d_job_deallocate((void *)&cpu_job);
	goto fail_init;
}
```

`v3d_job_deallocate()` only does `kfree(*container)`, freeing the `cpu_job` struct itself. The separately allocated query arrays pointed to by `cpu_job->timestamp_query.queries` and `cpu_job->performance_query.queries` are leaked. The `fail_init` label jumps past the cleanup that would free them.

The same issue exists in the extension failure path:

```c
ret = v3d_get_extensions(file_priv, args->extensions, &se, cpu_job);
if (ret) {
	v3d_job_deallocate((void *)&cpu_job);
	goto fail_init;
}
```

`v3d_get_extensions()` can succeed at allocating queries for one extension, then fail on a subsequent extension, returning an error. The partial allocations are not freed before `v3d_job_deallocate()` destroys the container struct. (This particular leak also exists in the original code — it's pre-existing.)

**Issue 3 (wrong tree base): patch conflicts with drm-next.**

The current drm-next `fail:` label includes:

```c
fail:
	v3d_job_cleanup((void *)cpu_job);
	v3d_job_cleanup((void *)csd_job);
	v3d_job_cleanup(clean_job);
	v3d_put_multisync_post_deps(&se);
	kvfree(cpu_job->timestamp_query.queries);
	kvfree(cpu_job->performance_query.queries);
```

The patch's context hunk doesn't show the `kvfree` lines, confirming it's based on an older tree. Note that the existing `kvfree` lines on drm-next are themselves a use-after-free bug — they access `cpu_job->timestamp_query.queries` after `v3d_job_cleanup()` may have already freed `cpu_job` via `kref_put` → `v3d_job_free` → `kfree(job)`.

**Suggested approach:**

1. Keep `v3d_job_free` as the kref callback (not `v3d_cpu_job_free`).
2. Move `v3d_job_init()` before the `job_type` validation, as this patch does.
3. Free query arrays *before* calling `v3d_job_cleanup()` in the `fail` path (fixing the existing use-after-free).
4. In the `fail_init` path, also free the query arrays before calling `v3d_job_deallocate()`.

---
Generated by Claude Code Patch Reviewer