Re: [PATCH v3 3/3] drm/v3d: Use write_to_buffer() helper in performance query copy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The V3D CPU job execution functions do not validate userspace-provided offsets, strides, and query counts against the GEM buffer size, allowing trivial out-of-bounds kernel memory reads and writes.
--

commit bccb8e9eddf193e4beeaa67eb00038b0dc9bc030
Author: Maíra Canal <mcanal@igalia.com>

drm/v3d: Use write_to_buffer() helper in performance query copy

This commit updates the performance query result copy logic to use the
write_to_buffer() helper. This replaces the open-coded 32-bit and 64-bit
selection loops with a single simplified loop.

> diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
> index e950cee31bcba..21ab9cd836856 100644
> --- a/drivers/gpu/drm/v3d/v3d_sched.c
> +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> @@ -605,15 +605,8 @@ v3d_write_performance_query_result(struct v3d_cpu_job *job, void *data,
>  
>  		v3d_perfmon_stop(v3d, perfmon, true);
>  
> -		if (job->copy.do_64bit) {
> -			for (j = 0; j < perfmon->ncounters; j++)
> -				write_to_buffer_64(data, offset + j,
> -						   perfmon->values[j]);
> -		} else {
> -			for (j = 0; j < perfmon->ncounters; j++)
> -				write_to_buffer_32(data, offset + j,
> -						   perfmon->values[j]);
> -		}
> +		for (j = 0; j < perfmon->ncounters; j++)
> +			write_to_buffer(data, offset + j, job->copy.do_64bit, perfmon->values[j]);

[Severity: High]
This is a pre-existing issue, but does this code properly validate the bounds
of the destination buffer before writing?

Looking at v3d_write_performance_query_result() and the surrounding job
execution functions like v3d_copy_performance_query(), it appears the offset,
stride, and count values provided by userspace are used without being checked
against the GEM object size.

For example, when calculating the initial data pointer and incrementing it:

    data = ((u8 *)bo->vaddr) + copy->offset;
    for (int i = 0; i < performance_query->count; i++) {
        // ...
        data += copy->stride;
    }

The same pattern seems to exist in v3d_copy_query_results() for timestamp
queries:

    data = ((u8 *)bo->vaddr) + copy->offset;

Since these userspace-provided parameters are added directly to the buffer's
kernel virtual address, could this allow an out-of-bounds memory access if
large offset, stride, or count parameters are passed via the CPU job ioctl?

>  
>  		v3d_perfmon_put(perfmon);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-v3d-cpu-job-fixes-v3-0-bc51b1f3eeb5@igalia.com?part=3