Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [Critical] User-controlled offsets, strides, and counts are used to access Buffer Object (BO) memory without any bounds checking, leading to arbitrary out-of-bounds kernel memory reads and writes. - [Critical] The function `v3d_get_bo_vaddr()` does not check if `vmap()` returns NULL, which leads to a subsequent NULL pointer dereference. -- commit d8a5e25eb7cfc120706c0d07ce2e2d8392b8454f Author: Maíra Canal <[email protected]> drm/v3d: Use write_to_buffer() helper in performance query copy The copy of performance query results to the output buffer is refactored to use the write_to_buffer() helper instead of open-coding the 32-bit/64-bit selection. This replaces two nearly identical loops. > diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c > index 53deae1a2eda9..17751c8d7f163 100644 > --- a/drivers/gpu/drm/v3d/v3d_sched.c > +++ b/drivers/gpu/drm/v3d/v3d_sched.c > @@ -594,15 +594,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: Critical] This is a pre-existing issue, but does this write out of bounds? Userspace submits a CPU job (e.g., V3D_CPU_JOB_TYPE_COPY_PERFORMANCE_QUERY) providing offset and stride via DRM ioctls. These values are parsed without being validated against the actual allocated size of the target BO. In v3d_submit.c:v3d_get_cpu_copy_performance_query_params(): job->copy.offset = copy.offset; job->copy.stride = copy.stride; The unvalidated offset is used to compute the base write pointer in v3d_sched.c:v3d_copy_performance_query(): data = ((u8 *)bo->vaddr) + copy->offset; This loop then blindly writes to data while incrementing the pointer by copy->stride without checking boundaries, which can lead to arbitrary out-of-bounds kernel memory reads and writes. Any unprivileged userspace process that can open the DRM device can trigger this via DRM_IOCTL_V3D_SUBMIT_CPU. A similar unvalidated out-of-bounds access is present in other CPU jobs like indirect CSD: v3d_sched.c:v3d_rewrite_csd_job_wg_counts_from_indirect(): wg_counts = (uint32_t *)(bo->vaddr + indirect_csd->offset); > > v3d_perfmon_put(perfmon); > } [Severity: Critical] This is a pre-existing issue, but does v3d_get_bo_vaddr() dereference a NULL pointer when vmap() fails? When the system is under memory pressure, vmap() can fail and return NULL. However, v3d_get_bo_vaddr() assigns this NULL pointer directly to bo->vaddr without checking: v3d_bo.c:v3d_get_bo_vaddr(): bo->vaddr = vmap(obj->pages, obj->base.size >> PAGE_SHIFT, VM_MAP, pgprot_writecombine(PAGE_KERNEL)); Callers compute offsets from bo->vaddr and proceed to read or write to it without any NULL checks. For example: v3d_sched.c:v3d_copy_performance_query(): v3d_get_bo_vaddr(bo); data = ((u8 *)bo->vaddr) + copy->offset; This is reachable by an untrusted userspace program triggering CPU jobs via DRM ioctls while the system is under memory pressure. -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
