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

Pre-existing issues:
- [High] Missing ttm_bo_unreserve in the '!vma' error path causes a permanent 
WW mutex lock leak.
- [High] Direct return on trycnt > 100000 bypasses all cleanup, leaking WW 
mutex locks, the WW acquire context, and GEM object references.
--

commit fc4adfd74f8e6257f658aeca598c47876610c9c5
Author: WenTao Liang <[email protected]>

drm/nouveau: fix GEM refcount leak in validate_init error paths

This commit adds drm_gem_object_put(gem) to error paths in validate_init()
to fix GEM object reference leaks when breaking out of the loop early.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
> b/drivers/gpu/drm/nouveau/nouveau_gem.c
> index 20dba02d6175b..c654256f4081e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_gem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
[ ... ]
> @@ -523,6 +524,7 @@ validate_init(struct nouveau_channel *chan, struct 
> drm_file *file_priv,
>                       if (!vma) {
>                               NV_PRINTK(err, cli, "vma not found!\n");
>                               ret = -EINVAL;
> +                             drm_gem_object_put(gem);
>                               break;
>                       }

[Severity: High]
This is a pre-existing issue, but does this break leave the buffer locked via
ttm_bo_reserve?

In validate_init(), if nouveau_vma_find() returns NULL for a successfully
reserved buffer, breaking here exits the loop before the buffer is added to
any list.

Because it is not on a list, validate_fini() will not unreserve it, which
appears to cause a permanent WW mutex lock leak for this GEM object.

[Severity: High]
This is also a pre-existing issue, but does the retry limit check at the top
of validate_init() bypass cleanup?

drivers/gpu/drm/nouveau/nouveau_gem.c:validate_init() {
    ...
retry:
        if (++trycnt > 100000) {
                NV_PRINTK(err, cli, "%s failed and gave up.\n", __func__);
                return -EINVAL;
        }
    ...

If the WW reservation loop hits a deadlock (-EDEADLK), reserves the buffer
via the slowpath, jumps to the retry label, and then exceeds the trycnt limit,
this direct return bypasses the rest of the function.

Because it returns directly without calling validate_fini_no_ticket() or
cleaning up, does this leak the WW mutex locks, the WW acquire context, and
the GEM object references?

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

Reply via email to