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

Pre-existing issues:
- [High] The job submission ioctl can return an error to user-space after the 
job has already been successfully committed to hardware, violating the 'point 
of no return' principle.
--

--- Patch [3]: [PATCH 3/6] drm/tegra: Drop reference on postfence ---
commit 3dc23eb6eb77ce1c25e7c0e305986ed6dd337897
Author: Mikko Perttunen <[email protected]>

drm/tegra: Drop reference on postfence

This commit drops the extra fence reference by calling dma_fence_put()
after drm_syncobj_replace_fence() is used in the job submission path.

> diff --git a/drivers/gpu/drm/tegra/submit.c b/drivers/gpu/drm/tegra/submit.c
> index 3f20b5a244476..658396f2e9549 100644
> --- a/drivers/gpu/drm/tegra/submit.c
> +++ b/drivers/gpu/drm/tegra/submit.c
[ ... ]
> @@ -645,21 +645,22 @@ int tegra_drm_ioctl_channel_submit(struct drm_device 
> *drm, void *data,
>  
>       /* Submit job to hardware. */
>       err = host1x_job_submit(job);
>       if (err) {
>               SUBMIT_ERR(context, "host1x job submission failed: %d", err);
>               goto unpin_job;
>       }
>  
>       /* Return postfences to userspace and add fences to DMA reservations. */
>       args->syncpt.value = job->syncpt_end;
>  
>       if (syncobj) {
>               struct dma_fence *fence = host1x_fence_create(job->syncpt, 
> job->syncpt_end, true);
>               if (IS_ERR(fence)) {
>                       err = PTR_ERR(fence);
>                       SUBMIT_ERR(context, "failed to create postfence: %d", 
> err);
>                       goto put_job;
>               }

[Severity: High]
This is a pre-existing issue, but returning an error to user-space here
after host1x_job_submit() has already succeeded seems problematic.

If host1x_fence_create() fails, the error path (goto put_job) drops the local
job reference and returns the error to userspace. Because userspace receives
an error from the ioctl, it might incorrectly assume the job was not
submitted and could free or reuse the command and data buffers. 

Since the GPU is already asynchronously executing the job, could this lead to
the hardware accessing freed memory and causing IOMMU faults or memory
corruption?

>  
>               drm_syncobj_replace_fence(syncobj, fence);
> +             dma_fence_put(fence);
>       }
>  
>       goto put_job;

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=3

Reply via email to