Re: [PATCH 1/6] drm/tegra: Fix syncobj_in wait return value check

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] If `host1x_fence_create()` fails, it returns an error pointer which is passed to `drm_syncobj_replace_fence()` due to a missing early return, causing a kernel crash when dereferenced.
- [High] The initial reference to the `dma_fence` allocated by `host1x_fence_create()` is leaked on the success path.
--

--- Patch [1]: [PATCH 1/6] drm/tegra: Fix syncobj_in wait return value check ---
commit 250614410b887862e5e7b49b5d80c8b53c8aa353
Author: Mikko Perttunen <mperttunen@nvidia.com>

drm/tegra: Fix syncobj_in wait return value check

dma_fence_wait_timeout() returns the remaining timeout in jiffies on
success, zero on timeout, and a negative error code if interrupted. The
return value was assigned to 'err' and checked as if it were an errno,
the opposite of the correct logic.

Fix the code to use a properly typed variable and check each condition
correctly.

Closes: https://lore.kernel.org/all/aJM_te551jUwnRv7@stanley.mountain/

> diff --git a/drivers/gpu/drm/tegra/submit.c b/drivers/gpu/drm/tegra/submit.c
> index e5841857c9378..5dad6dc4eb8c3 100644
> --- a/drivers/gpu/drm/tegra/submit.c
> +++ b/drivers/gpu/drm/tegra/submit.c

[ ... ]

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential problem in
tegra_drm_ioctl_channel_submit() when handling syncobj_out:

	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);
		}

		drm_syncobj_replace_fence(syncobj, fence);
	}

If host1x_fence_create() fails, it returns an error pointer. Since there is
no early return or goto in the error handling block, the error pointer falls
through and is passed to drm_syncobj_replace_fence().

Looking at drm_syncobj_replace_fence() in drm_syncobj.c, it calls
dma_fence_get(fence) which checks 'if (fence)'. This would evaluate to true
for an error pointer, causing an attempt to increment the reference count on
the invalid pointer.

Should there be a 'goto put_job' or similar return path inside the
IS_ERR(fence) block to avoid this panic?

[Severity: High]
This is also a pre-existing issue, but in the same block of code, does
this leak the fence on the success path? 

host1x_fence_create() returns a new dma_fence with a reference count of 1.
drm_syncobj_replace_fence() then takes its own reference to the fence by
calling dma_fence_get().

However, I don't see a matching dma_fence_put(fence) in this function
after drm_syncobj_replace_fence(). Will the initial reference to the fence
be leaked for every successful ioctl call using syncobj_out?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-b4-host1x-syncobj-wait-v1-0-afa730410392@nvidia.com?part=1