Re: [PATCH v3 06/14] drm/v3d: Reject invalid syncobj handles in submit ioctls

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing bounds check on userspace-controlled `in_sync_count` and `out_sync_count`, potentially leading to soft lockups or unbounded memory allocation attempts.
--

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

drm/v3d: Reject invalid syncobj handles in submit ioctls

This commit modifies v3d_job_add_syncobjs() to properly propagate error
codes when a non-zero syncobj handle doesn't resolve, while continuing to
ignore zero handles.

> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
> index 8250376d104c5..0babe2e672660 100644
> --- a/drivers/gpu/drm/v3d/v3d_submit.c
> +++ b/drivers/gpu/drm/v3d/v3d_submit.c

[ ... ]

> @@ -208,11 +207,13 @@ v3d_job_add_syncobjs(struct v3d_job *job, struct drm_file *file_priv,
>  				return -EFAULT;
>  			}
>  
> -			ret = drm_sched_job_add_syncobj_dependency(&job->base,
> -								   file_priv, in.handle, 0);
> -			// TODO: Investigate why this was filtered out for the IOCTL.
> -			if (ret && ret != -ENOENT)
> -				return ret;
> +			/* Ignore syncobj if its handle is zero */
> +			if (in.handle) {
> +				ret = drm_sched_job_add_syncobj_dependency(&job->base,
> +									   file_priv, in.handle, 0);
> +				if (ret)
> +					return ret;
> +			}
>  		}
>  	}
>  

[Severity: High]
This is a pre-existing issue, but is se->in_sync_count bounded by any 
upper limit?

This code is executing inside a loop over the userspace-supplied 
se->in_sync_count:

v3d_job_add_syncobjs() {
    ...
    for (int i = 0; i < se->in_sync_count; i++) {
        struct drm_v3d_sem in;

        if (copy_from_user(&in, handle++, sizeof(in))) {
    ...
}

Since there is no cond_resched() in this loop, could a maliciously large 
in_sync_count value allow a user to monopolize the processor and cause a 
soft lockup?

Similarly, is se->out_sync_count bounded before it is used for memory 
allocation in v3d_get_multisync_post_deps()?

v3d_get_multisync_post_deps() {
    ...
    se->out_syncs = (struct v3d_submit_outsync *)
        kvmalloc_objs(struct v3d_submit_outsync, count);
    if (!se->out_syncs)
    ...
}

Could an unbounded out_sync_count lead to a massive memory allocation 
request, potentially exhausting physical memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-v3d-sched-misc-fixes-v3-0-d7114bba55a0@igalia.com?part=6