From 671b7825dbfe9ea6e3ad3001003aeee0df48d1b5 Mon Sep 17 00:00:00 2001 From: Michael Bommarito Date: Tue, 7 Jul 2026 18:13:34 -0400 Subject: [PATCH] drm/v3d: bound CPU-job query writes to their destination BO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The V3D_SUBMIT_CPU CPU jobs take user-supplied offsets and indices and consume them at exec time without checking that the accesses stay inside their 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. - INDIRECT_CSD reads three u32 work-group counts from bo[0] at a user-controlled offset, then writes each count back into the indirect BO at a user-controlled u32 index (wg_uniform_offsets[]). A render-node user (DRM_RENDER_ALLOW, no master, no capability) can make the handlers read or write past a BO's vmap mapping. Validate the full access extent against the BO size once the BOs are looked up, before the job is queued, rejecting out-of-range geometry with -EINVAL. The copy extent offset + (count - 1) * stride + write_size is computed in u64, mirroring the u8 * pointer arithmetic in the executors: (count - 1) * stride is a u32 * u32 product that is exact in u64, so one overflow check on the total guards the bound. The performance slot count and the bare timestamp, copy-source and indirect offsets are computed in u64 the same way, so a user value cannot wrap the comparison. Fixes: 18b8413b25b7 ("drm/v3d: Create a CPU job extension for a indirect CSD job") Fixes: 9ba0ff3e083f ("drm/v3d: Create a CPU job extension for the timestamp query job") Fixes: 34a101e64296 ("drm/v3d: Create a CPU job extension for the reset timestamp job") Fixes: 6745f3e44a20 ("drm/v3d: Create a CPU job extension to copy timestamp query to a buffer") Fixes: 209e8d2695ee ("drm/v3d: Create a CPU job extension for the copy performance query job") Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Michael Bommarito Reviewed-by: Maíra Canal Signed-off-by: Maíra Canal Link: https://patch.msgid.link/20260707221334.3854433-1-michael.bommarito@gmail.com --- drivers/gpu/drm/v3d/v3d_submit.c | 125 +++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c index d32defa7da73..5c05f1ea24bc 100644 --- a/drivers/gpu/drm/v3d/v3d_submit.c +++ b/drivers/gpu/drm/v3d/v3d_submit.c @@ -5,6 +5,7 @@ */ #include +#include #include #include @@ -1377,6 +1378,126 @@ 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, u64 write_size) +{ + u64 last; + + if (!count) + return 0; + + /* + * The executors walk a u8 * cursor, so the furthest written byte is + * offset + (count - 1) * stride + write_size, matching the pointer + * arithmetic in v3d_copy_query_results()/v3d_copy_performance_query(). + * (count - 1) * stride is a u32 * u32 product that is exact in u64, + * and offset + write_size stays far below the u64 range, so a single + * overflow check guards the total. + */ + last = write_size + offset; + if (check_add_overflow((u64)(count - 1) * stride, last, &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; + u64 slots, write_size; + u32 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]); + + slots = (u64)job->performance_query.nperfmons * + DRM_V3D_MAX_PERF_COUNTERS; + if (copy->availability_bit) + slots = max(slots, + (u64)job->performance_query.ncounters + 1); + + write_size = slots * elem; + return v3d_check_copy_extent(dev, dst->base.base.size, + copy->offset, copy->stride, + job->performance_query.count, + write_size); + case V3D_CPU_JOB_TYPE_INDIRECT_CSD: { + struct v3d_indirect_csd_info *indirect_csd = &job->indirect_csd; + + /* 3 is the three dimensions (x, y, z) of the workgroup counts. */ + src = to_v3d_bo(job->base.bo[0]); + if ((u64)indirect_csd->offset + 3 * sizeof(u32) > + src->base.base.size) + goto err_range; + + dst = to_v3d_bo(indirect_csd->indirect); + for (i = 0; i < 3; i++) { + u32 uidx = indirect_csd->wg_uniform_offsets[i]; + + /* + * 0xffffffff means "skip this rewrite", so the exec + * path never writes that index and it needs no check. + */ + if (uidx != 0xffffffff && + (u64)uidx * sizeof(u32) + sizeof(u32) > + dst->base.base.size) + goto err_range; + } + return 0; + } + 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 @@ -1439,6 +1560,10 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data, ret = v3d_lookup_bos(&submit, args->bo_handles, args->bo_handle_count); if (ret) goto fail; + + ret = v3d_cpu_job_bounds_check(cpu_job); + if (ret) + goto fail; } if (cpu_job->job_type == V3D_CPU_JOB_TYPE_INDIRECT_CSD) {