[PATCH 0/2] drm/v3d: bound CPU-job copy-query writes to the destination BO

[PATCH 0/2] drm/v3d: bound CPU-job copy-query writes to the destination BO

[Michael Bommarito]

The V3D_SUBMIT_CPU COPY_TIMESTAMP_QUERY and COPY_PERFORMANCE_QUERY
extensions take a destination offset, per-query stride and query count
from userspace and consume them at job exec without checking the write
extent against the destination BO. A render-node user (DRM_RENDER_ALLOW,
no master, no capability) can drive a controlled-offset out-of-bounds
write past the BO's vmap mapping; the timestamp variant also reads each
result from an unchecked offset into the second BO.

Patch 1 validates the full write extent against the BO size before the
job is queued, using check_*_overflow() so a u32 product cannot wrap.
Patch 2 adds a KUnit reproducer the maintainer may take or drop.

Reproduced under KASAN via the KUnit over a shmem-backed BO: the trigger
faults on stock and is rejected at submit on patched; two in-bounds
controls pass on both.

Michael Bommarito (2):
  drm/v3d: validate copy-query buffer bounds against destination BO size
  drm/v3d: add KUnit reproducer for the copy-query out-of-bounds write

 drivers/gpu/drm/v3d/Kconfig                |  10 ++
 drivers/gpu/drm/v3d/v3d_copy_query_kunit.c | 172 +++++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_drv.h              |   3 +
 drivers/gpu/drm/v3d/v3d_sched.c            |   4 +
 drivers/gpu/drm/v3d/v3d_submit.c           |  86 +++++++++++
 5 files changed, 275 insertions(+)
 create mode 100644 drivers/gpu/drm/v3d/v3d_copy_query_kunit.c


base-commit: 5200f5f493f79f14bbdc349e402a40dfb32f23c8
-- 
2.53.0

[PATCH 1/2] drm/v3d: validate copy-query buffer bounds against destination BO size

[Michael Bommarito]

The V3D_SUBMIT_CPU COPY_TIMESTAMP_QUERY and COPY_PERFORMANCE_QUERY
extensions store a user-supplied destination offset, per-query stride and
query count on the job and consume them at exec without checking that
offset + (count - 1) * stride + per-query write size stays inside the
destination BO. The copy then writes counter values and the availability
bit past the BO's vmap mapping; the timestamp variant also reads each
result from an unchecked offset into the second BO. A render-node user
(DRM_RENDER_ALLOW, no master, no capability) controls the offset.

Validate the full write extent against the destination BO size once the
BOs are looked up, before the job is queued, rejecting overflow or
out-of-range geometry with -EINVAL; the per-query write size is computed
with check_*_overflow() so a u32 product cannot wrap the bound, and the
timestamp source offsets are bounded in the same pass. A KUnit reproducer
follows.

Fixes: 6745f3e44a20 ("drm/v3d: Create a CPU job extension to copy timestamp query to a buffer")
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-8
---
Reproduced under KASAN via a KUnit (patch 2) driving the real
v3d_copy_query_results() over a shmem-backed BO; a copy offset at the BO
size writes past the one-page vmap mapping:

  BUG: KASAN: vmalloc-out-of-bounds in v3d_copy_query_results+0x807/0x900

The trigger faults on stock and is rejected at submit time on the patched
tree; two in-bounds controls pass on both. The performance-query copy
shares the bound; its write values come from perfmon counters and were
checked by source review rather than this hardware-free run.

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

diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index ee4512db294b3..23e19dacfdce2 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -1246,6 +1246,88 @@ 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 span, last;
+
+	if (!count)
+		return 0;
+
+	if (check_mul_overflow(stride, count - 1, &span) ||
+	    check_add_overflow(span, write_size, &span) ||
+	    check_add_overflow(span, offset, &last) ||
+	    last > bo_size) {
+		drm_dbg(dev, "CPU job copy buffer exceeds the destination BO.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Bound the copy-query CPU-job writes; the exec-time copy does not. */
+static int
+v3d_cpu_job_check_copy_bounds(struct v3d_cpu_job *job)
+{
+	struct drm_device *dev = &job->base.v3d->drm;
+	struct v3d_copy_query_results_info *copy = &job->copy;
+	u32 elem = copy->do_64bit ? sizeof(u64) : sizeof(u32);
+	struct v3d_bo *bo, *timestamp;
+	u32 slots, write_size;
+	int i;
+
+	switch (job->job_type) {
+	case V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY:
+		bo = to_v3d_bo(job->base.bo[0]);
+		timestamp = to_v3d_bo(job->base.bo[1]);
+
+		slots = copy->availability_bit ? 2 : 1;
+		if (check_mul_overflow(slots, elem, &write_size))
+			return -EINVAL;
+
+		if (v3d_check_copy_extent(dev, bo->base.base.size, copy->offset,
+					  copy->stride, job->timestamp_query.count,
+					  write_size))
+			return -EINVAL;
+
+		for (i = 0; i < job->timestamp_query.count; i++) {
+			u32 end;
+
+			if (check_add_overflow(job->timestamp_query.queries[i].offset,
+					       (u32)sizeof(u64), &end) ||
+			    end > timestamp->base.base.size) {
+				drm_dbg(dev, "CPU job timestamp query offset exceeds the BO.\n");
+				return -EINVAL;
+			}
+		}
+		return 0;
+	case V3D_CPU_JOB_TYPE_COPY_PERFORMANCE_QUERY:
+		bo = to_v3d_bo(job->base.bo[0]);
+
+		if (check_mul_overflow(job->performance_query.nperfmons,
+				       (u32)DRM_V3D_MAX_PERF_COUNTERS, &slots))
+			return -EINVAL;
+		if (copy->availability_bit) {
+			u32 avail_slots;
+
+			if (check_add_overflow(job->performance_query.ncounters,
+					       1u, &avail_slots))
+				return -EINVAL;
+			slots = max(slots, avail_slots);
+		}
+		if (check_mul_overflow(slots, elem, &write_size))
+			return -EINVAL;
+
+		return v3d_check_copy_extent(dev, bo->base.base.size, copy->offset,
+					     copy->stride, job->performance_query.count,
+					     write_size);
+	default:
+		return 0;
+	}
+}
+
 /**
  * v3d_submit_cpu_ioctl() - Submits a CPU job to the V3D.
  * @dev: DRM device
@@ -1317,6 +1399,10 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
 		if (ret)
 			goto fail;
 
+		ret = v3d_cpu_job_check_copy_bounds(cpu_job);
+		if (ret)
+			goto fail;
+
 		ret = v3d_lock_bo_reservations(&cpu_job->base, &acquire_ctx);
 		if (ret)
 			goto fail;
-- 
2.53.0

[PATCH 2/2] drm/v3d: add KUnit reproducer for the copy-query out-of-bounds write

[Michael Bommarito]

Add a KUnit suite (CONFIG_DRM_V3D_COPY_QUERY_KUNIT_TEST, builds under
COMPILE_TEST) that drives the real v3d_copy_query_results() CPU-job
handler over a shmem-backed BO and exercises the destination-buffer
bounds the previous patch adds. Under KASAN the trigger case reproduces
the vmalloc-out-of-bounds write on the stock tree and passes once the
bounds patch is applied; two in-bounds controls pass on both trees.

Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-8
---
The COPY_TIMESTAMP_QUERY path is hardware-free: with an unsignalled query
the result read is skipped and only the availability bit is written, so the
copy runs on CPU memory alone. The trigger uses a copy offset at the BO
size, so the write lands past the one-page vmap mapping and KASAN
(CONFIG_KASAN_VMALLOC) reports a vmalloc-out-of-bounds write; the two
in-bounds controls prove the synthesised setup is not itself the cause.
With patch 1 applied the trigger is rejected by the submit-time gate
(exposed for the test) and the copy never runs. x86_64, COMPILE_TEST; the
write is plain CPU memory and not architecture specific.

 drivers/gpu/drm/v3d/Kconfig                |  10 ++
 drivers/gpu/drm/v3d/v3d_copy_query_kunit.c | 172 +++++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_drv.h              |   3 +
 drivers/gpu/drm/v3d/v3d_sched.c            |   4 +
 drivers/gpu/drm/v3d/v3d_submit.c           |   2 +-
 5 files changed, 190 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/v3d/v3d_copy_query_kunit.c

diff --git a/drivers/gpu/drm/v3d/Kconfig b/drivers/gpu/drm/v3d/Kconfig
index ce62c5908e1db..f293eb95a73c8 100644
--- a/drivers/gpu/drm/v3d/Kconfig
+++ b/drivers/gpu/drm/v3d/Kconfig
@@ -11,3 +11,13 @@ config DRM_V3D
 	  Choose this option if you have a system that has a Broadcom
 	  V3D 3.x or newer GPUs. SoCs supported include the BCM2711,
 	  BCM7268 and BCM7278.
+
+config DRM_V3D_COPY_QUERY_KUNIT_TEST
+	bool "KUnit test for V3D CPU-job copy-query bounds"
+	depends on DRM_V3D && KUNIT=y
+	select DRM_KUNIT_TEST_HELPERS
+	help
+	  Builds a KUnit suite that drives the V3D CPU-job copy-timestamp-query
+	  handler over a real shmem-backed BO to exercise the destination-buffer
+	  bounds. Used to reproduce the copy-query out-of-bounds write under
+	  KASAN. If in doubt, say N.
diff --git a/drivers/gpu/drm/v3d/v3d_copy_query_kunit.c b/drivers/gpu/drm/v3d/v3d_copy_query_kunit.c
new file mode 100644
index 0000000000000..4296449de7230
--- /dev/null
+++ b/drivers/gpu/drm/v3d/v3d_copy_query_kunit.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * KUnit coverage for the V3D CPU-job copy-query OOB write. Drives the real
+ * v3d_copy_query_results() (COPY_TIMESTAMP_QUERY) over a shmem-backed v3d_bo:
+ * a copy->offset at the BO size makes the availability write land past the
+ * one-page vmap, which KASAN reports. A NULL fence skips the result read, so
+ * no V3D engine is needed (Level 2). Two in-bounds controls pass on both.
+ * Included from v3d_sched.c.
+ */
+#include <linux/sizes.h>
+
+#include <kunit/test.h>
+
+#include <drm/drm_drv.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_kunit_helpers.h>
+#include <drm/drm_syncobj.h>
+
+/* One page per BO; the OOB write lands just past this vmap mapping. */
+#define V3D_COPY_TEST_BO_SIZE	PAGE_SIZE
+
+struct v3d_copy_test_ctx {
+	struct drm_device *drm;
+	struct v3d_bo *dst;			/* to_v3d_bo(job->base.bo[0]) */
+	struct v3d_bo *ts;			/* to_v3d_bo(job->base.bo[1]) */
+	struct drm_gem_object *bo_array[2];
+	struct drm_syncobj syncobj[4];		/* fence == NULL -> unavailable */
+	struct v3d_timestamp_query queries[4];
+	struct v3d_cpu_job job;
+};
+
+/* Standard shmem teardown; we have no v3d_dev for v3d_free_object(). */
+static void v3d_copy_test_free_bo(void *p)
+{
+	struct v3d_bo *bo = p;
+
+	if (bo->vaddr)
+		v3d_put_bo_vaddr(bo);
+	if (refcount_read(&bo->base.pages_pin_count))
+		drm_gem_shmem_unpin(&bo->base);
+	drm_gem_shmem_free(&bo->base);
+}
+
+static struct v3d_bo *
+v3d_copy_test_make_bo(struct kunit *test, struct drm_device *drm)
+{
+	struct drm_gem_shmem_object *shmem;
+	struct drm_gem_object *obj;
+	struct v3d_bo *bo;
+	int ret;
+
+	/* v3d_create_object sizes the alloc for struct v3d_bo (bo->vaddr). */
+	shmem = drm_gem_shmem_create(drm, V3D_COPY_TEST_BO_SIZE);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, shmem);
+
+	obj = &shmem->base;
+	bo = to_v3d_bo(obj);
+
+	/* Real page backing for v3d_get_bo_vaddr()'s vmap(). */
+	ret = drm_gem_shmem_pin(shmem);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	ret = kunit_add_action_or_reset(test, v3d_copy_test_free_bo, bo);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	return bo;
+}
+
+/* Build a COPY_TIMESTAMP_QUERY job and run the real copy handler. */
+static void v3d_copy_test_run(struct kunit *test, u32 offset, u32 stride,
+			      u32 count)
+{
+	struct v3d_copy_test_ctx *c;
+	struct drm_device *drm;
+	struct device *dev;
+
+	dev = drm_kunit_helper_alloc_device(test);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+
+	drm = __drm_kunit_helper_alloc_drm_device(test, dev,
+						  sizeof(*drm), 0,
+						  DRIVER_GEM);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
+
+	/* Route BO creation through the driver hook (devm drm_driver is writable). */
+	((struct drm_driver *)drm->driver)->gem_create_object = v3d_create_object;
+
+	c = kunit_kzalloc(test, sizeof(*c), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, c);
+	c->drm = drm;
+
+	c->dst = v3d_copy_test_make_bo(test, drm);
+	c->ts = v3d_copy_test_make_bo(test, drm);
+
+	c->bo_array[0] = &c->dst->base.base;
+	c->bo_array[1] = &c->ts->base.base;
+
+	/* fence == NULL: result read skipped; only the availability bit is written. */
+	KUNIT_ASSERT_LE(test, count, (u32)ARRAY_SIZE(c->queries));
+	for (u32 i = 0; i < count; i++) {
+		c->syncobj[i].fence = NULL;
+		c->queries[i].offset = 0;	/* in bounds for the source BO */
+		c->queries[i].syncobj = &c->syncobj[i];
+	}
+
+	c->job.job_type = V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY;
+	c->job.base.bo = c->bo_array;
+	c->job.base.bo_count = 2;
+	c->job.timestamp_query.queries = c->queries;
+	c->job.timestamp_query.count = count;
+
+	/* drm_device is the first member of v3d_dev; the cast resolves &v3d->drm back. */
+	c->job.base.v3d = (struct v3d_dev *)c->drm;
+
+	c->job.copy.do_64bit = false;		/* 4-byte writes */
+	c->job.copy.do_partial = false;
+	c->job.copy.availability_bit = true;	/* drives the index-1 write */
+	c->job.copy.offset = offset;
+	c->job.copy.stride = stride;
+
+#if defined(V3D_CPU_JOB_COPY_BOUNDS_CHECKED)
+	/* Patched tree: run the same submit-time gate; out-of-range is rejected before the copy. */
+	if (v3d_cpu_job_check_copy_bounds(&c->job)) {
+		KUNIT_EXPECT_GT_MSG(test, c->job.copy.offset + count * stride,
+				    (u32)V3D_COPY_TEST_BO_SIZE,
+				    "bounds gate rejected an in-bounds geometry");
+		return;
+	}
+#endif
+
+	v3d_copy_query_results(&c->job);
+}
+
+/* Control: one query at offset 0; availability bit at byte 4, in bounds. */
+static void v3d_copy_query_control_inbounds(struct kunit *test)
+{
+	v3d_copy_test_run(test, 0, 8, 1);
+}
+
+/* Control: two queries, stride 8; second availability bit at byte 12, in bounds. */
+static void v3d_copy_query_control_inbounds_multi(struct kunit *test)
+{
+	v3d_copy_test_run(test, 0, 8, 2);
+}
+
+/* Trigger: copy->offset == BO size; write past the vmap mapping (stock OOB, patched rejected). */
+static void v3d_copy_query_trigger_oob(struct kunit *test)
+{
+	v3d_copy_test_run(test, V3D_COPY_TEST_BO_SIZE, 8, 1);
+}
+
+static struct kunit_case v3d_copy_query_cases[] = {
+	KUNIT_CASE(v3d_copy_query_control_inbounds),
+	KUNIT_CASE(v3d_copy_query_control_inbounds_multi),
+	KUNIT_CASE(v3d_copy_query_trigger_oob),
+	{}
+};
+
+static struct kunit_suite v3d_copy_query_suite = {
+	.name = "v3d_copy_query",
+	.test_cases = v3d_copy_query_cases,
+};
+
+kunit_test_suite(v3d_copy_query_suite);
+
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 6a3cad9334398..a4b772385070f 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -583,6 +583,9 @@ int v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
 			 struct drm_file *file_priv);
 int v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
 			 struct drm_file *file_priv);
+/* Exposed for the copy-query KUnit; its presence marks the bounds patch applied. */
+#define V3D_CPU_JOB_COPY_BOUNDS_CHECKED 1
+int v3d_cpu_job_check_copy_bounds(struct v3d_cpu_job *job);
 
 /* v3d_irq.c */
 int v3d_irq_init(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 1855ef5b3b5fe..7884863376702 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -901,3 +901,7 @@ v3d_sched_fini(struct v3d_dev *v3d)
 			drm_sched_fini(&v3d->queue[q].sched);
 	}
 }
+
+#if IS_ENABLED(CONFIG_DRM_V3D_COPY_QUERY_KUNIT_TEST)
+#include "v3d_copy_query_kunit.c"
+#endif
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 23e19dacfdce2..ec0b94ada73ff 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -1268,7 +1268,7 @@ v3d_check_copy_extent(struct drm_device *dev, size_t bo_size,
 }
 
 /* Bound the copy-query CPU-job writes; the exec-time copy does not. */
-static int
+int
 v3d_cpu_job_check_copy_bounds(struct v3d_cpu_job *job)
 {
 	struct drm_device *dev = &job->base.v3d->drm;
-- 
2.53.0

Re: [PATCH 1/2] drm/v3d: validate copy-query buffer bounds against destination BO size

[Maíra Canal]

Hi Michael,

On 14/06/26 10:10, Michael Bommarito wrote:
> The V3D_SUBMIT_CPU COPY_TIMESTAMP_QUERY and COPY_PERFORMANCE_QUERY

If you are doing it for V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY and
V3D_CPU_JOB_TYPE_COPY_PERFORMANCE_QUERY, you should also do it for
V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY and
V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY.

> extensions store a user-supplied destination offset, per-query stride and
> query count on the job and consume them at exec without checking that
> offset + (count - 1) * stride + per-query write size stays inside the
> destination BO. The copy then writes counter values and the availability
> bit past the BO's vmap mapping; the timestamp variant also reads each
> result from an unchecked offset into the second BO. A render-node user
> (DRM_RENDER_ALLOW, no master, no capability) controls the offset.
> 
> Validate the full write extent against the destination BO size once the
> BOs are looked up, before the job is queued, rejecting overflow or
> out-of-range geometry with -EINVAL; the per-query write size is computed
> with check_*_overflow() so a u32 product cannot wrap the bound, and the
> timestamp source offsets are bounded in the same pass. A KUnit reproducer
> follows.
> 
> Fixes: 6745f3e44a20 ("drm/v3d: Create a CPU job extension to copy timestamp query to a buffer")
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> Assisted-by: Claude:claude-opus-4-8
> ---
> Reproduced under KASAN via a KUnit (patch 2) driving the real
> v3d_copy_query_results() over a shmem-backed BO; a copy offset at the BO
> size writes past the one-page vmap mapping:
> 
>    BUG: KASAN: vmalloc-out-of-bounds in v3d_copy_query_results+0x807/0x900
> 
> The trigger faults on stock and is rejected at submit time on the patched
> tree; two in-bounds controls pass on both. The performance-query copy
> shares the bound; its write values come from perfmon counters and were
> checked by source review rather than this hardware-free run.
> 
>   drivers/gpu/drm/v3d/v3d_submit.c | 86 ++++++++++++++++++++++++++++++++
>   1 file changed, 86 insertions(+)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
> index ee4512db294b3..23e19dacfdce2 100644
> --- a/drivers/gpu/drm/v3d/v3d_submit.c
> +++ b/drivers/gpu/drm/v3d/v3d_submit.c
> @@ -1246,6 +1246,88 @@ 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 span, last;
> +

You don't need the variable "span".

> +	if (!count)
> +		return 0;
> +
> +	if (check_mul_overflow(stride, count - 1, &span) ||
> +	    check_add_overflow(span, write_size, &span) ||
> +	    check_add_overflow(span, offset, &last) ||
> +	    last > bo_size) {
> +		drm_dbg(dev, "CPU job copy buffer exceeds the destination BO.\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Bound the copy-query CPU-job writes; the exec-time copy does not. */

This comment is a bit confusing.

> +static int
> +v3d_cpu_job_check_copy_bounds(struct v3d_cpu_job *job)

How about "v3d_cpu_job_bounds_check"?

> +{
> +	struct drm_device *dev = &job->base.v3d->drm;
> +	struct v3d_copy_query_results_info *copy = &job->copy;
> +	u32 elem = copy->do_64bit ? sizeof(u64) : sizeof(u32);
> +	struct v3d_bo *bo, *timestamp;
> +	u32 slots, write_size;
> +	int i;
> +
> +	switch (job->job_type) {
> +	case V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY:
> +		bo = to_v3d_bo(job->base.bo[0]);
> +		timestamp = to_v3d_bo(job->base.bo[1]);
> +
> +		slots = copy->availability_bit ? 2 : 1;
> +		if (check_mul_overflow(slots, elem, &write_size))

How could this ever overflow? slots is 1 or 2; elem is 4 or 8. The
maximum product is 2 * 8 = 16. That cannot overflow any integer type.

> +			return -EINVAL;
> +
> +		if (v3d_check_copy_extent(dev, bo->base.base.size, copy->offset,
> +					  copy->stride, job->timestamp_query.count,
> +					  write_size))
> +			return -EINVAL;
> +
> +		for (i = 0; i < job->timestamp_query.count; i++) {
> +			u32 end;
> +
> +			if (check_add_overflow(job->timestamp_query.queries[i].offset,
> +					       (u32)sizeof(u64), &end) ||
> +			    end > timestamp->base.base.size) {
> +				drm_dbg(dev, "CPU job timestamp query offset exceeds the BO.\n");
> +				return -EINVAL;
> +			}
> +		}
> +		return 0;

I have the impression that this function is over complicating something
simple. This could be as simple as:

dst = to_v3d_bo(job->base.bo[0]);
src = to_v3d_bo(job->base.bo[1]);

for (i = 0; i < tquery->count; i++) {
     if (tquery->queries[i].offset + sizeof(u64) > src->base.base.size)
         goto err_range;
}

write_size = (copy->availability_bit ? 2 : 1) * (u64)elem;
return v3d_check_copy_extent(dev, dst->base.base.size,
                              copy->offset, copy->stride,
                              tquery->count, write_size);

The same logic of simplification also apply to
V3D_CPU_JOB_TYPE_COPY_PERFORMANCE_QUERY (with its own caveats).

About your second patch (the KUnit reproducer), it's a nice test to
reproduce this issue, but for me, it's a no-go in terms of upstreaming.
This is a very simple issue, not related to any v3d functional
regression. So, I'd prefer not to take the KUnit test to the branch.
Regarding this fix, a v2 addressing the issues pointed out is welcome.

Thanks for your patch.

Best regards,
- Maíra

Re: [PATCH 1/2] drm/v3d: validate copy-query buffer bounds against destination BO size

[Michael Bommarito]

On Sun, Jun 14, 2026 at 4:12 PM Maíra Canal <mcanal@igalia.com> wrote:

> If you are doing it for V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY and
> V3D_CPU_JOB_TYPE_COPY_PERFORMANCE_QUERY, you should also do it for
> V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY and
> V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY.

OK, will do!

> How could this ever overflow? slots is 1 or 2; elem is 4 or 8. The
> maximum product is 2 * 8 = 16. That cannot overflow any integer type.
> ...
>      if (tquery->queries[i].offset + sizeof(u64) > src->base.base.size)

You were definitely right about the first check being overkill, but I
think we do need to think about 32-bit for this spot.  Let me  know in
v2 if I missed something

> About your second patch (the KUnit reproducer), it's a nice test to
> reproduce this issue, but for me, it's a no-go in terms of upstreaming.
> This is a very simple issue, not related to any v3d functional
> regression. So, I'd prefer not to take the KUnit test to the branch.
> Regarding this fix, a v2 addressing the issues pointed out is welcome.

Sounds good.  I am trying to send them by default after getting yelled
at by Greg for not including unit tests in every patch he sees :)

V2 coming up soon

Thanks,
Mike

Re: [PATCH 2/2] drm/v3d: add KUnit reproducer for the copy-query out-of-bounds write

[Julian Braha]

Hi Michael,

On 6/14/26 14:11, Michael Bommarito wrote:

> +config DRM_V3D_COPY_QUERY_KUNIT_TEST
> +	bool "KUnit test for V3D CPU-job copy-query bounds"
> +	depends on DRM_V3D && KUNIT=y
> +	select DRM_KUNIT_TEST_HELPERS
> +	help
> +	  Builds a KUnit suite that drives the V3D CPU-job copy-timestamp-query
> +	  handler over a real shmem-backed BO to exercise the destination-buffer
> +	  bounds. Used to reproduce the copy-query out-of-bounds write under
> +	  KASAN. If in doubt, say N.
Could you hide this option from the Kconfig frontend
when KUNIT_ALL_TESTS is enabled?

E.g.:
bool "KUnit test for V3D CPU-job copy-query bounds" if !KUNIT_ALL_TESTS

From Documentation/dev-tools/kunit/style.rst:
"This Kconfig entry must:
<snip>
* be visible only if ``CONFIG_KUNIT_ALL_TESTS`` is not enabled."

- Julian Braha