Re: [PATCH v11 31/74] drm/rockchip: dw_hdmi_qp: Fix invalid drvdata access in PM ops

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Using `devm_*` APIs in the component `bind` callback coupled with a `drmm_` allocated context causes resource leaks on bind retries and use-after-free vulnerabilities.
--

commit 78a45b3c5f7568988aeb46e28d8dcd405d6b493f
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Fix invalid drvdata access in PM ops

This commit defers publishing the driver data until the component bind
callback completes successfully, and clears it during unbind. This ensures
that the PM callbacks can rely on a single null check to avoid accessing
invalid or freed state.

Closes: https://lore.kernel.org/all/20260702151550.8E14D1F000E9@smtp.kernel.org/
Closes: https://lore.kernel.org/all/20260723014811.1F9F01F000E9@smtp.kernel.org/

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 9c4cf68d79d50..646a4cb968309 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -683,8 +683,6 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  	if (ret)
>  		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder");
>  
> -	platform_set_drvdata(pdev, hdmi);
> -

[Severity: High]
This is a pre-existing issue, but does using devm_ allocations in a component
bind callback risk resource leaks and use-after-free bugs?

The component bind function registers resources like clocks, GPIO, PHY, and
IRQs using devm_ APIs on the component's platform device (dev). Since these
resources are tied to the platform device's lifetime, they won't be freed
when the component is unbound or when bind fails and is retried.

For example, earlier in dw_hdmi_qp_rockchip_bind():

	ret = devm_request_threaded_irq(dev, hdmi->hpd_irq,
					cfg->ctrl_ops->hardirq_callback,
					cfg->ctrl_ops->irq_callback,
					IRQF_NO_AUTOEN, "dw-hdmi-qp-hpd",
					hdmi);

This registers the IRQ against the platform device's lifetime, but passes
the hdmi pointer as context. The hdmi structure is allocated using
drmm_kzalloc(), which ties it to the DRM master device's lifetime.

If the DRM master is unbound or fails to probe, hdmi is freed, but the IRQ
remains registered. If an interrupt fires after the master is unbound, can
the active IRQ handler in dw_hdmi_qp_rk3588_irq() dereference the freed
hdmi pointer?

Furthermore, do error paths after IRQ registration return without canceling
the delayed work or freeing the devm_ resources, accumulating leaks on
repeated bind failures?

>  	hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data);
>  	if (IS_ERR(hdmi->hdmi))
>  		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi),

[ ... ]

> @@ -704,6 +704,8 @@ static void dw_hdmi_qp_rockchip_unbind(struct device *dev,
>  {
>  	struct rockchip_hdmi_qp *hdmi = dev_get_drvdata(dev);
>  
> +	dev_set_drvdata(dev, NULL);
> +
>  	cancel_delayed_work_sync(&hdmi->hpd_work);
>  }

[Severity: High]
This is also a pre-existing issue, but does cleaning up the delayed work
here still leave the IRQ registered against the platform device?

Because devm_request_threaded_irq() was used, the IRQ isn't freed during
unbind. Could a subsequent interrupt trigger a use-after-free by queuing
the work embedded within the already freed hdmi struct?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-dw-hdmi-qp-scramb-v11-0-bc12954a0688@collabora.com?part=31