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

Pre-existing issues:
- [High] Missing WW mutex unreserve and lock leak on `!vma` error path
- [High] Direct return on deadlock retry limit bypasses all cleanup
--

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

fix: drm/nouveau: validate_init: break paths after drm_gem_object_lookup leak 
GEM reference

This commit fixes GEM object reference leaks in nouveau's validate_init().
It adds drm_gem_object_put() on two error paths where the validation loop
breaks 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 isn't a bug introduced by this patch, but does breaking out of the
loop here leak the WW mutex lock?

At this point in the loop, ttm_bo_reserve() has already successfully
completed. Because the loop breaks before adding the buffer to the tracking
lists (vram_list, gart_list, etc.), validate_fini() will not see the object
to unreserve it.

Could this lead to a WW mutex deadlock and a potential stack use-after-free
when op->ticket goes out of scope?

[Severity: High]
This is another pre-existing issue in the same function, but does the
deadlock avoidance retry logic 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 loop hits this retry limit and returns directly, it skips
ww_acquire_fini() and fails to unreserve the slow-reserved buffer (res_bo).
Can this result in a permanent leak of GEM references, the WW ticket
context, and WW mutex locks?

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

Reply via email to