Re: [PATCH v10 32/69] drm/rockchip: dw_hdmi_qp: Use local dev variable consistently in bind()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free and resource leaks due to devm_ allocations in component bind callback.
- [High] Pending delayed work is not canceled on error paths, leading to Use-After-Free.
- [High] Missing cleanup for dw_hdmi_qp_bind() on the error path leads to resource and bridge leaks.
--

--- Patch [32]: [PATCH v10 32/69] drm/rockchip: dw_hdmi_qp: Use local dev variable consistently in bind() ---
commit 43fdb5eca012528d707389ce53b62eb4132b9532
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Use local dev variable consistently in bind()

Replace indirect struct device accesses via hdmi->dev and pdev->dev with
the local dev parameter already available in dw_hdmi_qp_rockchip_bind(),
for consistency and readability.

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 8b48558deb3e7..36043bf82273c 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c

[ ... ]

> @@ -670,7 +669,7 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  	if (irq < 0)
>  		return irq;
>  
> -	ret = devm_request_threaded_irq(hdmi->dev, irq,
> +	ret = devm_request_threaded_irq(dev, irq,
>  					cfg->ctrl_ops->hardirq_callback,
>  					cfg->ctrl_ops->irq_callback,
>  					IRQF_SHARED, "dw-hdmi-qp-hpd",
>  					hdmi);

[Severity: High]
This isn't a bug introduced by this patch, but does the use of devm_ here
cause a use-after-free and resource leak on component unbind?

The hdmi context is allocated via drmm_kzalloc, tying its lifetime to the
DRM master device. However, devm_request_threaded_irq ties the HPD IRQ
handler to the platform device's lifecycle, not the component bind/unbind
lifecycle.

If the component is unbound (e.g. the DRM master is removed), drmm will free
the hdmi context, but the IRQ remains active. A subsequent HPD interrupt
will trigger the handler, which dereferences the freed hdmi pointer. Also,
every bind retry leaks devm resources like clocks and GPIOs since they
aren't tied to the bind lifecycle.

>  	if (ret)
>  		return ret;
>  
>  	drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs);
>  	ret = drmm_encoder_init(drm, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
>  	if (ret)
> -		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder\n");
> +		return dev_err_probe(dev, ret, "Failed to init encoder\n");

[Severity: High]
This is a pre-existing issue, but does this error path lead to a
use-after-free?

The HPD IRQ is requested earlier and can fire immediately, scheduling
hdmi->hpd_work. If drmm_encoder_init() fails here, we return directly
without calling cancel_delayed_work_sync().

The DRM core handles the component bind failure by tearing down the DRM
device, causing drmm to free the hdmi structure. The pending delayed work
will then execute and dereference the freed hdmi pointer.

>  
>  	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),
> +		return dev_err_probe(dev, PTR_ERR(hdmi->hdmi),
>  				     "Failed to bind dw-hdmi-qp\n");
>  
>  	connector = drm_bridge_connector_init(drm, encoder);
>  	if (IS_ERR(connector))
> -		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
> +		return dev_err_probe(dev, PTR_ERR(connector),
>  				     "Failed to init bridge connector\n");

[Severity: High]
This isn't a bug introduced by this patch, but does this error path
permanently leak devm-managed bridges?

dw_hdmi_qp_bind() allocates state and registers a DRM bridge using devm_
functions tied to the platform device. If drm_bridge_connector_init() fails,
we return directly without calling dw_hdmi_qp_unbind() or cleaning up the
bridge.

Because the resources are managed by devm_ tied to the platform device, they
won't be freed when the component bind fails. Every retry of the component
bind will allocate new memory and register duplicate DRM bridges in the
global list.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-dw-hdmi-qp-scramb-v10-0-294364b2cf15@collabora.com?part=32