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

Pre-existing issues:
- [Medium] Memory leak of live_active object and preallocated barrier nodes in 
live_active_barrier error path.
- [Medium] Memory leak of heap-allocated fence `submit` on multiple error paths 
inside live_hwsp_read.
--

commit 7023619d481f3e0e660c2bb0c854c33e4fd279ed
Author: WenTao Liang <[email protected]>

drm/i915: fix kref leak in __live_active_setup error path

When heap_fence_create fails, the early error path calls kfree(active)
directly instead of __live_put(active), bypassing the kref_put path that
would call i915_active_fini for proper resource teardown. This skips
cleanup of the i915_active state while the initial kref from kref_init
remains unbalanced.

Suggested-by: Greg KH <[email protected]>
Fixes: 5361db1a33c7 ("drm/i915: Track i915_active using debugobjects")
Cc: [email protected]
Signed-off-by: WenTao Liang <[email protected]>

> diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c 
> b/drivers/gpu/drm/i915/selftests/i915_active.c
> index 9fea2fabeac48..8ec7859da7626 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_active.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_active.c
> @@ -91,7 +91,7 @@ __live_active_setup(struct drm_i915_private *i915)
>  
>       submit = heap_fence_create(GFP_KERNEL);
>       if (!submit) {
> -             kfree(active);
> +             __live_put(active);
>               return ERR_PTR(-ENOMEM);
>       }
>  

[ ... ]

> @@ -221,11 +221,11 @@ static int live_active_barrier(void *arg)
>       for_each_uabi_engine(engine, i915) {
>               err = i915_active_acquire_preallocate_barrier(&active->base,
>                                                             engine);
>               if (err)
>                       break;
>  
>               i915_active_acquire_barrier(&active->base);
>       }
>  
>       i915_active_release(&active->base);
>       if (err)
>               goto out;

[Severity: Medium]
This is a pre-existing issue, but does this loop exit on error and jump to out:,
skipping the __i915_active_wait() call?

If i915_active_acquire_preallocate_barrier() fails after at least one successful
iteration, the preallocated proto-nodes added in previous iterations are never
flushed or signaled.

This means active->base.count never reaches 0, which prevents __live_retire from
being called and leaks the initial kref.

> diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c 
> b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> --- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
> @@ -1026,10 +1026,10 @@ static int live_hwsp_read(void *arg)
>               do {
>                       struct i915_sw_fence *submit;
>                       struct i915_request *rq;
>                       u32 hwsp, dummy;
>  
>                       submit = heap_fence_create(GFP_KERNEL);
>                       if (!submit) {
>                               err = -ENOMEM;
>                               goto out;
>                       }
>  
>                       err = create_watcher(&watcher[0], engine, SZ_4K);
>                       if (err)
>                               goto out;

[Severity: Medium]
This isn't a bug introduced by this patch, but can this leak the heap-allocated
submit fence?

If any subsequent setup step inside the loop fails (like create_watcher() here),
the code jumps to the out: label outside the loop.

Since submit is a local variable inside the loop, and the cleanup code at out:
does not free it, this looks like a permanent leak.

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

Reply via email to