* [bug report] drm/tegra: Implement job submission part of new UAPI
@ 2025-08-06 11:42 Dan Carpenter
2025-08-22 3:07 ` Mikko Perttunen
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2025-08-06 11:42 UTC (permalink / raw)
To: Mikko Perttunen; +Cc: linux-tegra
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.
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] drm/tegra: Implement job submission part of new UAPI
2025-08-06 11:42 [bug report] drm/tegra: Implement job submission part of new UAPI Dan Carpenter
@ 2025-08-22 3:07 ` Mikko Perttunen
0 siblings, 0 replies; 2+ messages in thread
From: Mikko Perttunen @ 2025-08-22 3:07 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-tegra
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-08-22 3:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06 11:42 [bug report] drm/tegra: Implement job submission part of new UAPI Dan Carpenter
2025-08-22 3:07 ` Mikko Perttunen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox