From: Michael Bommarito <michael.bommarito@gmail.com>
To: Melissa Wen <mwen@igalia.com>, Maira Canal <mcanal@igalia.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
Iago Toral Quiroga <itoral@igalia.com>,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: [PATCH v4] drm/v3d: bound CPU-job query writes to their destination BO
Date: Tue, 7 Jul 2026 18:13:34 -0400 [thread overview]
Message-ID: <20260707221334.3854433-1-michael.bommarito@gmail.com> (raw)
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 <michael.bommarito@gmail.com>
---
v4:
- Rebase onto drm-misc-next (Maira Canal): this is a fix, but a
reasonably large change and not a critical bug.
- Reconcile with the "drm/v3d: Miscellaneous fixes" series [1]. Drop the
INDIRECT_CSD NULL indirect-BO reject: [1] patch 2/3 ("drm/v3d: Reject
invalid indirect BO handle in indirect CSD setup") already returns
-ENOENT for a bad handle at parse time, so that check is redundant.
This patch assumes that series lands first. The out-of-bounds
offset/index checks here are still novel and unfixed by [1].
- Declare the loop index i as u32 rather than int (Sashiko, Maira
Canal): a signed index could go negative once count exceeds INT_MAX.
- Trim the TIMESTAMP_QUERY comment and restore the v2 wording of the
COPY_PERFORMANCE_QUERY comment (Maira Canal).
- INDIRECT_CSD: name the rewrite target dst and the counts BO src, add
the "3 == x, y, z workgroup dims" and 0xffffffff-sentinel comments,
and rename the local to indirect_csd (Maira Canal).
[1] https://lore.kernel.org/dri-devel/20260703-v3d-cpu-job-fixes-v3-0-bc51b1f3eeb5@igalia.com/
v3: https://lore.kernel.org/all/20260617125811.3692951-1-michael.bommarito@gmail.com/
v2: https://lore.kernel.org/all/20260614211644.217116-1-michael.bommarito@gmail.com/
v1: https://lore.kernel.org/all/20260614131058.2525157-1-michael.bommarito@gmail.com/
drivers/gpu/drm/v3d/v3d_submit.c | 126 +++++++++++++++++++++++++++++++
1 file changed, 126 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index ee2ac2540ed50..277823b774bed 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -4,6 +4,8 @@
* Copyright (C) 2023 Raspberry Pi
*/
+#include <linux/overflow.h>
+
#include <drm/drm_print.h>
#include <drm/drm_syncobj.h>
@@ -1300,6 +1302,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
@@ -1364,6 +1486,10 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
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) {
ret = v3d_setup_csd_jobs_and_bos(&submit, &cpu_job->indirect_csd.args,
NULL);
base-commit: 1acd235bc8b8cf62da1a2b497f3e8c5cf01a2205
--
2.53.0
next reply other threads:[~2026-07-07 22:13 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-07 22:13 Michael Bommarito [this message]
2026-07-07 22:25 ` [PATCH v4] drm/v3d: bound CPU-job query writes to their destination BO sashiko-bot
2026-07-08 12:22 ` Maíra Canal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260707221334.3854433-1-michael.bommarito@gmail.com \
--to=michael.bommarito@gmail.com \
--cc=airlied@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=itoral@igalia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mcanal@igalia.com \
--cc=mripard@kernel.org \
--cc=mwen@igalia.com \
--cc=simona@ffwll.ch \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.