Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [Critical] Pre-existing double-free vulnerability in CPU job extension error handling. - [Critical] Missing bounds checks on userspace-provided offsets in v3d CPU jobs allows arbitrary kernel memory access. - [High] Missing check for vmap failure when mapping GEM objects for CPU jobs. -- commit 8d6290edb4b042b28b895df1c1037e680a314bfc Author: Maíra Canal <[email protected]> drm/v3d: Reject invalid indirect BO handle in indirect CSD setup This commit fixes a NULL pointer dereference in the v3d driver by rejecting invalid indirect buffer object handles during indirect CSD job setup. It returns -ENOENT if drm_gem_object_lookup() fails to find the object. > diff --git a/drivers/gpu/drm/v3d/v3d_submit.c > b/drivers/gpu/drm/v3d/v3d_submit.c > index 3d6582dfb1bfc..122b93eaa8bf2 100644 > --- a/drivers/gpu/drm/v3d/v3d_submit.c > +++ b/drivers/gpu/drm/v3d/v3d_submit.c > @@ -601,6 +601,8 @@ v3d_get_cpu_indirect_csd_params(struct drm_file > *file_priv, > sizeof(indirect_csd.wg_uniform_offsets)); > > info->indirect = drm_gem_object_lookup(file_priv, > indirect_csd.indirect); [Severity: Critical] This is a pre-existing issue, but does this code lack bounds checking on the userspace-provided offsets before using them to access kernel memory? In v3d_get_cpu_indirect_csd_params(), offsets like indirect_csd.offset and indirect_csd.wg_uniform_offsets are copied from the userspace extension struct without any validation against the bounds of the GEM object. When the job executes in v3d_rewrite_csd_job_wg_counts_from_indirect(): wg_counts = (uint32_t *)(bo->vaddr + indirect_csd->offset); ... u32 uniform_idx = indirect_csd->wg_uniform_offsets[i]; ((uint32_t *)indirect->vaddr)[uniform_idx] = wg_counts[i]; Can a malicious userspace application supply arbitrarily large offsets to read from or write to arbitrary kernel memory addresses relative to the mapped BO? Similar issues seem to exist in v3d_timestamp_query() as well: value_addr = ((u8 *)bo->vaddr) + timestamp_query->queries[i].offset; *((u64 *)value_addr) = i == 0 ? ktime_get_ns() : 0ull; [Severity: High] This isn't a bug introduced by this patch, but does the driver fail to handle vmap() returning NULL when mapping these GEM objects? In v3d_get_bo_vaddr(): bo->vaddr = vmap(obj->pages, obj->base.size >> PAGE_SHIFT, VM_MAP, pgprot_writecombine(PAGE_KERNEL)); If vmap() fails, bo->vaddr is NULL. CPU job execution functions like v3d_rewrite_csd_job_wg_counts_from_indirect() unconditionally assume bo->vaddr is valid: v3d_get_bo_vaddr(bo); v3d_get_bo_vaddr(indirect); wg_counts = (uint32_t *)(bo->vaddr + indirect_csd->offset); Can this lead to a NULL pointer dereference if the system is under memory pressure or if a very large BO is submitted? [Severity: Critical] This is also a pre-existing issue, but is there a double-free vulnerability in the CPU job extension error handling? When parsing CPU job parameters in v3d_get_cpu_timestamp_query_params(), if a failure occurs, the error path calls: error: v3d_timestamp_query_info_free(&job->timestamp_query, i); return err; The helper v3d_timestamp_query_info_free() frees the queries array: kvfree(query_info->queries); but it does not set the pointer to NULL. If the error propagates to v3d_submit_cpu_ioctl(), it jumps to its error path and calls v3d_submit_cleanup_jobs() -> v3d_cpu_job_free(). This function unconditionally calls the free helper again: v3d_timestamp_query_info_free(&job->timestamp_query, job->timestamp_query.count); Can this allow a malicious userspace application to trigger a double free of kernel memory? > + if (!info->indirect) > + return -ENOENT; > > return 0; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
