Re: [bug report] drm/tegra: Implement job submission part of new UAPI

[Mikko Perttunen <mperttunen@nvidia.com>]

On Wednesday, August 6, 2025 8:42 PM Dan Carpenter wrote:
> Hello Mikko Perttunen,
> 
> Commit 13abe0bb15ce ("drm/tegra: Implement job submission part of new
> UAPI") from Jun 10, 2021 (linux-next), leads to the following Smatch
> static checker warning:
> 
> 	drivers/gpu/drm/tegra/submit.c:541 tegra_drm_ioctl_channel_submit()
> 	warn: save dma_fence_wait_timeout() returns to signed long
> 
> drivers/gpu/drm/tegra/submit.c
>     509 int tegra_drm_ioctl_channel_submit(struct drm_device *drm, void
> *data, 510                                    struct drm_file *file)
>     511 {
>     512         struct tegra_drm_file *fpriv = file->driver_priv;
>     513         struct drm_tegra_channel_submit *args = data;
>     514         struct tegra_drm_submit_data *job_data;
>     515         struct drm_syncobj *syncobj = NULL;
>     516         struct tegra_drm_context *context;
>     517         struct host1x_job *job;
>     518         struct gather_bo *bo;
>     519         u32 i;
>     520         int err;
>     521
>     522         mutex_lock(&fpriv->lock);
>     523
>     524         context = xa_load(&fpriv->contexts, args->context);
>     525         if (!context) {
>     526                 mutex_unlock(&fpriv->lock);
>     527                 pr_err_ratelimited("%s: %s: invalid channel context
> '%#x'", __func__, 528                                    current->comm,
> args->context); 529                 return -EINVAL;
>     530         }
>     531
>     532         if (args->syncobj_in) {
>     533                 struct dma_fence *fence;
>     534
>     535                 err = drm_syncobj_find_fence(file, args->syncobj_in,
> 0, 0, &fence); 536                 if (err) {
>     537                         SUBMIT_ERR(context, "invalid syncobj_in
> '%#x'", args->syncobj_in); 538                         goto unlock;
>     539                 }
>     540
> --> 541                 err = dma_fence_wait_timeout(fence, true,
> msecs_to_jiffies(10000)); 542                 dma_fence_put(fence);
>     543                 if (err) {
> 
> This checking is wrong.  dma_fence_wait_timeout() returns a non-zero
> positive value on success so tt should be something like:
> 
> 	if (err <= 0) {
> 		err = err ?: -ETIMEDOUT;
> 
> Except dma_fence_wait_timeout() can also return custom error codes so maybe
> something more complicated is needed.  This bug seems pretty severe, so it's
> strange that it hasn't been detected in testing.

Yeah.. This has probably never been tested except when initially written, or 
maybe not even then. Considering this is still in staging, the best course is 
probably to just remove the syncobj stuff for now.

Thanks for the report --
Mikko

> 
> 
>     544                         SUBMIT_ERR(context, "wait for syncobj_in
> timed out"); 545                         goto unlock;
>     546                 }
>     547         }
>     548
> 
> [ snip ]
> 
>     674
>     675         kfree(job_data);
>     676 put_bo:
>     677         gather_bo_put(&bo->base);
>     678 unlock:
>     679         if (syncobj)
>     680                 drm_syncobj_put(syncobj);
>     681
>     682         mutex_unlock(&fpriv->lock);
>     683         return err;
>     684 }
> 
> regards,
> dan carpenter