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

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

[Guangshuo Li]

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

If the autosuspend delay is set to a negative value while autosuspend
is enabled, the runtime PM core increments usage_count to prevent
runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
during teardown, this reference is not dropped.

The documentation for pm_runtime_use_autosuspend() also notes that it
is important to undo it with pm_runtime_dont_use_autosuspend() at
driver exit time, unless runtime PM was initially enabled with
devm_pm_runtime_enable().

Add the missing pm_runtime_dont_use_autosuspend() call to the driver
remove path.

This issue was found by manual code inspection.

Fixes: ace01e2af387 ("drm/tegra: gr2d/gr3d: Contain PM in the gr*d_probe/gr*d_remove")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/gpu/drm/tegra/gr2d.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
index 892e3450b281..95e8a14a9648 100644
--- a/drivers/gpu/drm/tegra/gr2d.c
+++ b/drivers/gpu/drm/tegra/gr2d.c
@@ -296,6 +296,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);
 }
-- 
2.43.0

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

[sashiko-bot]

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