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