Re: [PATCH] drm/tegra: gr2d: fix autosuspend cleanup during teardown

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The commit message falsely claims that gr2d_probe() calls pm_runtime_use_autosuspend(), and adds a redundant cleanup to gr2d_remove() based on this false premise.

Pre-existing issues:
- [High] Unbinding the gr2d driver while a DRM channel is open causes a Use-After-Free when the DRM file is later closed by userspace.
--

--- Patch [1]: [PATCH] drm/tegra: gr2d: fix autosuspend cleanup during teardown ---
commit 9b1fea12d4967b1c2b4a943c3958df804b6cd6a8
Author: Guangshuo Li <lgs201920130244@gmail.com>

    drm/tegra: gr2d: fix autosuspend cleanup during teardown

This commit adds a pm_runtime_dont_use_autosuspend() call to the driver's
remove function. It attempts to fix a leaked usage count reference that the
commit message states was taken in gr2d_probe().

>     gr2d_probe() calls pm_runtime_use_autosuspend(), but gr2d_remove()
>     does not call the matching pm_runtime_dont_use_autosuspend() during
>     teardown.

[Severity: Medium]
Is this description accurate?

Looking at the code, gr2d_probe() does not call pm_runtime_use_autosuspend().

Furthermore, pm_runtime_dont_use_autosuspend() is already properly called
during teardown inside gr2d_exit(), which is reached via
host1x_client_unregister().

Does this patch add a redundant cleanup based on an incorrect premise?

> diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
> index 21f4dd0fa6aff..8063e06b0b87a 100644
> --- a/drivers/gpu/drm/tegra/gr2d.c
> +++ b/drivers/gpu/drm/tegra/gr2d.c
> @@ -293,6 +293,7 @@ static void gr2d_remove(struct platform_device *pdev)
>  {
>  	struct gr2d *gr2d = platform_get_drvdata(pdev);
>  
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	host1x_client_unregister(&gr2d->client.base);
>  }

[Severity: High]
This is a pre-existing issue, but does unbinding the driver here
cause a use-after-free if a DRM channel remains open?

If an administrator unbinds the tegra-gr2d driver while userspace has a DRM
channel open, gr2d_remove() will execute. The struct gr2d driver data will
then be freed by devres.

When userspace subsequently closes the DRM file descriptor,
tegra_drm_postclose() calls tegra_drm_context_free(), which dereferences
the freed client ops:

drivers/gpu/drm/tegra/drm.c:tegra_drm_context_free() {
    ...
    context->client->ops->close_channel(context);
    ...
}

Should the DRM layer be coordinated with to revoke active contexts before the
device structure is destroyed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808140622.2887873-1-lgs201920130244@gmail.com?part=1