[PATCH 0/2] drm/v3d: Fix CPU job teardown bugs

[PATCH 0/2] drm/v3d: Fix CPU job teardown bugs

[Maíra Canal]

This series addresses two independent teardown bugs in V3D's CPU job
submission path:

 - PATCH 1: A use-after-free on the ioctl's fail label, where the timestamp
   and performance query arrays are freed through cpu_job after
   v3d_job_cleanup() has already released the job. It consolidates the CPU
   job teardown into the job's kref destructor so a single code path covers
   both the scheduler .free_job and ioctl error paths.

 - PATCH 2: A GEM reference leak on indirect CSD jobs: the extra reference
   taken by v3d_get_cpu_indirect_csd_params() is never released. It adds
   the missing drop in v3d_cpu_job_free().

Claude was the one that identified these teardown bugs, although the fixes
were written by myself. Therefore, an Assisted-by tag was added to the
patches in conformance with the AI Coding Assistants [0] policy.

[0] https://docs.kernel.org/process/coding-assistants.html

Best regards,
- Maíra

---
Maíra Canal (2):
      drm/v3d: Fix use-after-free of CPU job query arrays on error path
      drm/v3d: Release indirect CSD GEM reference on CPU job free

 drivers/gpu/drm/v3d/v3d_sched.c  | 16 +---------------
 drivers/gpu/drm/v3d/v3d_submit.c | 22 +++++++++++++++++++---
 2 files changed, 20 insertions(+), 18 deletions(-)
---
base-commit: 0a9c56dd387605d17dabeedd9fdd2c4c1d0bab7b
change-id: 20260515-v3d-cpu-job-leaks-06c60f2af206

[PATCH 1/2] drm/v3d: Fix use-after-free of CPU job query arrays on error path

[Maíra Canal]

The CPU job ioctl's fail label calls kvfree() on cpu_job's timestamp and
performance query arrays after v3d_job_cleanup(), which drops the job's
last reference and frees cpu_job. Reading cpu_job at that point is a
use-after-free. Also, on the early v3d_job_init() failure path, it is a
NULL dereference, since v3d_job_deallocate() zeroes the local pointer.

In the success path, the arrays are released from the scheduler's
.free_job callback, but on the error path, they are freed manually, as
the job was never pushed to the scheduler. While the success path deals
with this correctly, the fail path doesn't.

On top of that, the manual kvfree() calls only free the array storage;
they don't drm_syncobj_put() the per-query syncobjs that
v3d_timestamp_query_info_free() and v3d_performance_query_info_free()
release on the success path. So the same fail path that triggers the
use-after-free also leaks one syncobj reference per query.

Unify the CPU job teardown into the CPU job's kref destructor, mirroring
v3d_render_job_free(). The scheduler's .free_job slot reverts to the
generic v3d_sched_job_free() and the fail label drops the manual
kvfree() calls, leaving a single teardown path that is reached from both
the scheduler and the ioctl error path. That removes the use-after-free,
the NULL dereference, and the syncobj leak by construction.

Fixes: 9ba0ff3e083f ("drm/v3d: Create a CPU job extension for the timestamp query job")
Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_sched.c  | 16 +---------------
 drivers/gpu/drm/v3d/v3d_submit.c | 19 ++++++++++++++++---
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 1855ef5b3b5f..94bf628dc91c 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -125,20 +125,6 @@ v3d_performance_query_info_free(struct v3d_performance_query_info *query_info,
 	}
 }
 
-static void
-v3d_cpu_job_free(struct drm_sched_job *sched_job)
-{
-	struct v3d_cpu_job *job = to_cpu_job(sched_job);
-
-	v3d_timestamp_query_info_free(&job->timestamp_query,
-				      job->timestamp_query.count);
-
-	v3d_performance_query_info_free(&job->performance_query,
-					job->performance_query.count);
-
-	v3d_job_cleanup(&job->base);
-}
-
 static void
 v3d_switch_perfmon(struct v3d_dev *v3d, struct v3d_job *job)
 {
@@ -830,7 +816,7 @@ static const struct drm_sched_backend_ops v3d_cache_clean_sched_ops = {
 
 static const struct drm_sched_backend_ops v3d_cpu_sched_ops = {
 	.run_job = v3d_cpu_job_run,
-	.free_job = v3d_cpu_job_free
+	.free_job = v3d_sched_job_free
 };
 
 static int
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index ee4512db294b..e3a6e7cc7bd5 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -123,6 +123,21 @@ v3d_render_job_free(struct kref *ref)
 	v3d_job_free(ref);
 }
 
+static void
+v3d_cpu_job_free(struct kref *ref)
+{
+	struct v3d_cpu_job *job = container_of(ref, struct v3d_cpu_job,
+					       base.refcount);
+
+	v3d_timestamp_query_info_free(&job->timestamp_query,
+				      job->timestamp_query.count);
+
+	v3d_performance_query_info_free(&job->performance_query,
+					job->performance_query.count);
+
+	v3d_job_free(ref);
+}
+
 void v3d_job_cleanup(struct v3d_job *job)
 {
 	if (!job)
@@ -1302,7 +1317,7 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
 	trace_v3d_submit_cpu_ioctl(&v3d->drm, cpu_job->job_type);
 
 	ret = v3d_job_init(v3d, file_priv, &cpu_job->base,
-			   v3d_job_free, 0, &se, V3D_CPU);
+			   v3d_cpu_job_free, 0, &se, V3D_CPU);
 	if (ret) {
 		v3d_job_deallocate((void *)&cpu_job);
 		goto fail;
@@ -1385,8 +1400,6 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
 	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);
 
 	return ret;
 }

-- 
2.54.0

Claude review: drm/v3d: Fix use-after-free of CPU job query arrays on error path

[Claude Code Review Bot]

Patch Review

**Analysis of the bug:** The old code had `v3d_cpu_job_free()` as a `drm_sched_backend_ops.free_job` callback in `v3d_sched.c`, which called `v3d_job_cleanup()` at the end. On the ioctl error path (`fail:` label in `v3d_submit_cpu_ioctl`), `v3d_job_cleanup()` was called first (which does `kref_put → kfree`), then the code tried to access `cpu_job->timestamp_query.queries` and `cpu_job->performance_query.queries` — a clear use-after-free. This analysis is correct.

**Fix approach:** The patch moves the cleanup logic from the scheduler's `.free_job` callback into a kref destructor (`v3d_cpu_job_free(struct kref *ref)` in `v3d_submit.c`), registered via `v3d_job_init()`. The scheduler now uses the generic `v3d_sched_job_free`, which calls `v3d_job_cleanup() → v3d_job_put() → kref_put(..., job->free)`, ultimately dispatching to the new `v3d_cpu_job_free`. This mirrors exactly how `v3d_render_job_free` works:

```c
static void
v3d_render_job_free(struct kref *ref)
{
	struct v3d_render_job *job = container_of(ref, struct v3d_render_job,
						  base.refcount);
	...
	v3d_job_free(ref);
}
```

The new function follows the same pattern:

```c
static void
v3d_cpu_job_free(struct kref *ref)
{
	struct v3d_cpu_job *job = container_of(ref, struct v3d_cpu_job,
					       base.refcount);

	v3d_timestamp_query_info_free(&job->timestamp_query,
				      job->timestamp_query.count);

	v3d_performance_query_info_free(&job->performance_query,
					job->performance_query.count);

	v3d_job_free(ref);
}
```

This is correct. The `v3d_timestamp_query_info_free` and `v3d_performance_query_info_free` functions both check for `query_info->queries` being non-NULL before iterating, so they're safe to call when no queries were set up (e.g., early error paths or CPU job types that don't use queries).

The removal of the manual `kvfree()` calls from the `fail:` label is safe because `v3d_job_cleanup()` will now trigger the full teardown via kref.

The `v3d_job_init()` call is correctly updated to pass `v3d_cpu_job_free` instead of `v3d_job_free`:

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

**No issues found.** This is a solid fix.

---
Generated by Claude Code Patch Reviewer

[PATCH 2/2] drm/v3d: Release indirect CSD GEM reference on CPU job free

[Maíra Canal]

v3d_get_cpu_indirect_csd_params() takes a reference to the indirect BO via
drm_gem_object_lookup() and stashes it in cpu_job->indirect_csd.indirect,
but nothing on the CPU job teardown path ever drops that reference.

Drop the extra reference in v3d_cpu_job_free(). The NULL check covers ioctl
errors before the lookup ran and CPU job types other than
V3D_CPU_JOB_TYPE_INDIRECT_CSD, which leave the field zero-initialised.

Fixes: 18b8413b25b7 ("drm/v3d: Create a CPU job extension for a indirect CSD job")
Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_submit.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index e3a6e7cc7bd5..3ddd53b6f437 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -135,6 +135,9 @@ v3d_cpu_job_free(struct kref *ref)
 	v3d_performance_query_info_free(&job->performance_query,
 					job->performance_query.count);
 
+	if (job->indirect_csd.indirect)
+		drm_gem_object_put(job->indirect_csd.indirect);
+
 	v3d_job_free(ref);
 }
 

-- 
2.54.0

Claude review: drm/v3d: Release indirect CSD GEM reference on CPU job free

[Claude Code Review Bot]

Patch Review

**Analysis of the bug:** `v3d_get_cpu_indirect_csd_params()` calls `drm_gem_object_lookup()` at `v3d_submit.c:497`:

```c
info->indirect = drm_gem_object_lookup(file_priv, indirect_csd.indirect);
```

`drm_gem_object_lookup()` takes a reference on the GEM object, but nowhere in the teardown path was `drm_gem_object_put()` called to release it. This is a straightforward reference leak.

**Fix:**

```c
if (job->indirect_csd.indirect)
	drm_gem_object_put(job->indirect_csd.indirect);
```

The NULL check is appropriate: for non-indirect-CSD job types, `indirect_csd.indirect` will be zero-initialized (the job struct is allocated with `kcalloc` in `v3d_job_allocate`). It also covers the case where `v3d_job_init()` fails before the extension parsing reaches the indirect CSD path.

The placement in `v3d_cpu_job_free()` (the kref destructor from patch 1) is the right spot — it ensures the reference is dropped on both the scheduler path and the ioctl error path.

**No issues found.** Clean and correct.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Fix CPU job teardown bugs

[Claude Code Review Bot]

Overall Series Review

Subject: drm/v3d: Fix CPU job teardown bugs
Author: =?utf-8?q?Ma=C3=ADra_Canal?= <mcanal@igalia.com>
Patches: 3
Reviewed: 2026-05-16T08:58:36.842986

---

This is a clean, well-motivated 2-patch series that fixes real teardown bugs in V3D's CPU job submission path. Both patches are correct, minimal, and well-structured.

**Patch 1** consolidates the CPU job teardown into the kref destructor (`v3d_cpu_job_free`), mirroring the existing `v3d_render_job_free` pattern. This eliminates a use-after-free on the ioctl error path where `cpu_job` was accessed after `v3d_job_cleanup()` had already released it, and also fixes a syncobj leak since the old manual `kvfree()` calls didn't call `drm_syncobj_put()` on each query entry.

**Patch 2** adds the missing `drm_gem_object_put()` for the indirect CSD BO reference acquired by `drm_gem_object_lookup()` in `v3d_get_cpu_indirect_csd_params()`.

The series is correctly ordered — patch 2 builds on the `v3d_cpu_job_free` kref destructor introduced in patch 1. The commit messages are clear and provide good analysis of the bugs.

**Verdict: Both patches look good for merging.** No issues found.

---

---
Generated by Claude Code Patch Reviewer