* Re: [PATCH] gpu: host1x: fix runtime PM reference leak on remove
2026-09-13 12:02 [PATCH] gpu: host1x: fix runtime PM reference leak on remove Guangshuo Li
@ 2026-09-13 12:18 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-09-13 12:18 UTC (permalink / raw)
To: Guangshuo Li; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Shared IRQ handler accesses hardware registers without PM checks, and runtime suspend powers down hardware without `synchronize_irq()`, risking system crashes.
- [High] `host1x_remove()` leaves the device in an invalid PM state (RPM_ACTIVE with hardware off) if children exist, causing bus hangs on driver rebind.
- [High] Resource leak (clocks left permanently on) in `host1x_probe` error path if child devices exist.
- [High] Use-After-Free risk due to allocating `struct device` objects as an array and freeing them synchronously in `host1x_memory_context_list_free`.
- [High] Use-After-Free risk due to missing `cancel_work_sync()` for `cdma->update_work` during CDMA de-initialization.
--
commit ee98077bd8be81a3568fe9ef94599ff2844511f0
Author: Guangshuo Li <lgs201920130244@gmail.com>
gpu: host1x: fix runtime PM reference leak on remove
The commit fixes a runtime PM reference leak in host1x_remove() by calling
pm_runtime_put_sync_suspend() before forcing the device into suspend. This
balances the reference acquired during host1x_probe().
> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
> index d2c64728f8043..bb6a1647e4db9 100644
> --- a/drivers/gpu/host1x/dev.c
> +++ b/drivers/gpu/host1x/dev.c
> @@ -729,6 +729,7 @@ static void host1x_remove(struct platform_device *pdev)
> host1x_unregister(host);
> host1x_debug_deinit(host);
>
> + pm_runtime_put_sync_suspend(&pdev->dev);
> pm_runtime_force_suspend(&pdev->dev);
[Severity: High]
This isn't a bug introduced by this patch, but does this leave the device in
an invalid PM state (RPM_ACTIVE with hardware off) if children exist?
In host1x_remove(), pm_runtime_put_sync_suspend() drops the usage counter
but fails to suspend because devm-managed children might exist
(child_count > 0). pm_runtime_force_suspend() is then called, which powers
down the hardware.
However, because child_count > 0, pm_runtime_need_not_resume() returns false,
causing pm_runtime_force_suspend() to leave the runtime state as RPM_ACTIVE.
When the driver is unbound and rebound, host1x_probe() calls
pm_runtime_resume_and_get(), which sees RPM_ACTIVE, skips the resume
callback, and accesses the powered-off hardware. Can this cause bus hangs on
driver rebind?
While reviewing the PM changes, I noticed a few other pre-existing issues in
the driver:
[Severity: High]
In drivers/gpu/host1x/dev.c:host1x_probe(), is there a resource leak in the
error path if child devices exist?
pm_put:
pm_runtime_put_sync_suspend(&pdev->dev);
pm_disable:
pm_runtime_disable(&pdev->dev);
This is a pre-existing issue, but if an error occurs in host1x_probe() after
devm_of_platform_populate() adds children, pm_runtime_put_sync_suspend()
fails to suspend with -EBUSY due to child_count > 0.
The driver exits probe, and devm cleans up the children, but the device is
left in RPM_ACTIVE with PM disabled, leaving hardware clocks permanently
enabled.
[Severity: High]
In drivers/gpu/host1x/intr.c:host1x_intr_init(), the interrupt handler is
registered as a shared interrupt:
err = devm_request_irq(host->dev, host->syncpt_irqs[i],
host->intr_op->isr, IRQF_SHARED,
"host1x_syncpt", &irq_data[i]);
This isn't a bug introduced by this patch, but doesn't this risk system
crashes?
If the device is runtime suspended and another device sharing the same IRQ
line triggers an interrupt, syncpt_thresh_isr() will execute and access
host1x registers without checking PM status:
reg = host1x_sync_readl(host,
HOST1X_SYNC_SYNCPT_THRESH_CPU0_INT_STATUS(i));
Additionally, host1x_runtime_suspend() powers down the hardware without
waiting for mid-flight IRQs via synchronize_irq().
[Severity: High]
This is a pre-existing issue, but does host1x_memory_context_list_free() in
drivers/gpu/host1x/context.c have a use-after-free risk?
for (i = 0; i < cdl->len; i++)
device_unregister(&cdl->devs[i].dev);
kfree(cdl->devs);
The device array is synchronously freed immediately after device_unregister().
If userspace holds a reference (e.g., via an open sysfs file), the memory is
freed while in use, and when the final reference is dropped, the driver core
will operate on freed memory.
[Severity: High]
This is a pre-existing issue, but does host1x_cdma_deinit() in
drivers/gpu/host1x/cdma.c risk a use-after-free by missing a work cleanup?
host1x_pushbuffer_destroy(pb);
host1x_hw_cdma_timeout_destroy(host1x, cdma);
return 0;
The function destroys CDMA resources and the channel memory is freed, but it
omits calling cancel_work_sync(&cdma->update_work).
If the work executes after the resources are freed, cdma_update_work() will
dereference freed memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260913120227.1559129-1-lgs201920130244@gmail.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread