Re: [PATCH 2/3] drm/v3d: Reject invalid indirect BO handle in indirect CSD setup

[sashiko-bot@kernel.org]

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 <mcanal@igalia.com>

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/20260610-v3d-cpu-job-fixes-v1-0-0d9c88989edc@igalia.com?part=2