[PATCH v2] drm/v3d: bound CPU-job query writes to their destination BO

[Michael Bommarito <michael.bommarito@gmail.com>]

The V3D_SUBMIT_CPU query CPU jobs store a user-supplied destination
offset (and, for the copy variants, a per-query stride and query count)
on the job and consume them at exec time without checking that the
writes stay inside the destination BO:

  - TIMESTAMP_QUERY and RESET_TIMESTAMP_QUERY write one u64 per query
    into bo[0] at a fully user-controlled per-query offset.
  - COPY_TIMESTAMP_QUERY copies one u64 per query into bo[0] at
    offset + i * stride, and reads each result from a user-controlled
    offset in the source bo[1].
  - COPY_PERFORMANCE_QUERY writes nperfmons * DRM_V3D_MAX_PERF_COUNTERS
    counter slots plus an availability slot into bo[0] at the same
    geometry.

None of these are bounded against the BO size, so a render-node user
(DRM_RENDER_ALLOW, no master, no capability) can make the handlers
write past the BO's vmap mapping.

Validate the full write extent against the destination BO size once the
BOs are looked up, before the job is queued, rejecting out-of-range or
overflowing geometry with -EINVAL. The copy extent
offset + (count - 1) * stride + write_size is accumulated with
check_*_overflow() so a u32 product cannot wrap the bound, and the
performance slot count is computed the same way since nperfmons and
ncounters are user values. The bare timestamp writes and the copy
source reads are a single fixed u64 slot per query and are bounded
directly.

Fixes: 9ba0ff3e083f ("drm/v3d: Create a CPU job extension for the timestamp query job")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2:
  - Extend the bound to the bare TIMESTAMP_QUERY and
    RESET_TIMESTAMP_QUERY job types, which also write one u64 per query
    into bo[0] at a user-controlled offset (Maira Canal).
  - Simplify v3d_cpu_job_bounds_check(): the timestamp source/dest u64
    slots are bounded directly without the can-never-overflow
    check_mul_overflow(); only the genuinely user-sized copy extent and
    the performance slot count keep check_*_overflow().
  - Drop the redundant local in v3d_check_copy_extent(); rename the gate
    to v3d_cpu_job_bounds_check(); reword the helper comments.
  - Drop the KUnit reproducer patch; the suite is kept out of tree.
  - Re-pin Fixes: to the earliest introducing commit (the timestamp
    query job) now that the bare timestamp writes are covered.

Reproduced under KASAN with an out-of-tree KUnit driving the real
handlers over shmem-backed BOs: with a query offset at the BO size the
stock tree reports a vmalloc-out-of-bounds write in each of
v3d_copy_query_results() (4 bytes), v3d_timestamp_query() (8 bytes) and
v3d_reset_timestamp_queries() (8 bytes); with this patch the submit-time
gate rejects the geometry and all in-bounds controls still pass. The
write is plain CPU memory and not architecture specific (x86_64,
COMPILE_TEST).

 drivers/gpu/drm/v3d/v3d_submit.c | 102 +++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index ee4512db294b3..fe11fd7e6e14a 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -1246,6 +1246,104 @@ static const unsigned int cpu_job_bo_handle_count[] = {
 	[V3D_CPU_JOB_TYPE_COPY_PERFORMANCE_QUERY] = 1,
 };
 
+/* Reject offset + (count - 1) * stride + write_size if it leaves the BO. */
+static int
+v3d_check_copy_extent(struct drm_device *dev, size_t bo_size,
+		      u32 offset, u32 stride, u32 count, u32 write_size)
+{
+	u32 last;
+
+	if (!count)
+		return 0;
+
+	if (check_mul_overflow(stride, count - 1, &last) ||
+	    check_add_overflow(last, write_size, &last) ||
+	    check_add_overflow(last, offset, &last) ||
+	    last > bo_size) {
+		drm_dbg(dev, "CPU job copy buffer exceeds the destination BO.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Reject a query CPU job whose writes would land outside their BO. */
+static int
+v3d_cpu_job_bounds_check(struct v3d_cpu_job *job)
+{
+	struct drm_device *dev = &job->base.v3d->drm;
+	struct v3d_timestamp_query_info *tquery = &job->timestamp_query;
+	struct v3d_copy_query_results_info *copy = &job->copy;
+	u32 elem = copy->do_64bit ? sizeof(u64) : sizeof(u32);
+	struct v3d_bo *dst, *src;
+	u32 slots, write_size;
+	int i;
+
+	switch (job->job_type) {
+	case V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY:
+	case V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY:
+		/* Each query writes one u64 timestamp slot into bo[0]. */
+		dst = to_v3d_bo(job->base.bo[0]);
+
+		for (i = 0; i < tquery->count; i++) {
+			if ((u64)tquery->queries[i].offset + sizeof(u64) >
+			    dst->base.base.size)
+				goto err_range;
+		}
+		return 0;
+	case V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY:
+		/* Copies one u64 per query from bo[1] into bo[0]. */
+		dst = to_v3d_bo(job->base.bo[0]);
+		src = to_v3d_bo(job->base.bo[1]);
+
+		for (i = 0; i < tquery->count; i++) {
+			if ((u64)tquery->queries[i].offset + sizeof(u64) >
+			    src->base.base.size)
+				goto err_range;
+		}
+
+		write_size = (copy->availability_bit ? 2 : 1) * elem;
+		return v3d_check_copy_extent(dev, dst->base.base.size,
+					     copy->offset, copy->stride,
+					     tquery->count, write_size);
+	case V3D_CPU_JOB_TYPE_COPY_PERFORMANCE_QUERY:
+		/*
+		 * Each query writes nperfmons * DRM_V3D_MAX_PERF_COUNTERS
+		 * counter slots into bo[0], plus an availability slot at
+		 * index ncounters. nperfmons and ncounters are user values,
+		 * so the slot count is computed overflow-safe.
+		 */
+		dst = to_v3d_bo(job->base.bo[0]);
+
+		if (check_mul_overflow(job->performance_query.nperfmons,
+				       (u32)DRM_V3D_MAX_PERF_COUNTERS, &slots))
+			goto err_range;
+
+		if (copy->availability_bit) {
+			u32 avail_slots;
+
+			if (check_add_overflow(job->performance_query.ncounters,
+					       1u, &avail_slots))
+				goto err_range;
+			slots = max(slots, avail_slots);
+		}
+
+		if (check_mul_overflow(slots, elem, &write_size))
+			goto err_range;
+
+		return v3d_check_copy_extent(dev, dst->base.base.size,
+					     copy->offset, copy->stride,
+					     job->performance_query.count,
+					     write_size);
+	default:
+		return 0;
+	}
+
+err_range:
+	drm_dbg(dev, "CPU job query offset exceeds the BO.\n");
+	return -EINVAL;
+}
+
 /**
  * v3d_submit_cpu_ioctl() - Submits a CPU job to the V3D.
  * @dev: DRM device
@@ -1317,6 +1415,10 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
 		if (ret)
 			goto fail;
 
+		ret = v3d_cpu_job_bounds_check(cpu_job);
+		if (ret)
+			goto fail;
+
 		ret = v3d_lock_bo_reservations(&cpu_job->base, &acquire_ctx);
 		if (ret)
 			goto fail;
-- 
2.53.0