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 <[email protected]>

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/[email protected]?part=6

Reply via email to