Re: [PATCH v10 31/69] drm/rockchip: dw_hdmi_qp: Add missing newlines in dev_err_probe() messages

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Using devm_ APIs on the component device in the bind callback causes resource leaks and a use-after-free on unbind/rebind.
- [High] The dw_hdmi_qp library lacks an unbind function, causing permanent resource leaks when componentized drivers unbind.
--

--- Patch [31]: [PATCH v10 31/69] drm/rockchip: dw_hdmi_qp: Add missing newlines in dev_err_probe() messages ---
commit 5dd7ce4a56ed7d72cce118d1d9700cdba6b578d8
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/rockchip: dw_hdmi_qp: Add missing newlines in dev_err_probe() messages

This commit adds missing trailing newlines to a couple of dev_err_probe()
calls in dw_hdmi_qp_rockchip_bind().

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index e9fda67b8ca94..8b48558deb3e7 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -681,12 +681,12 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  	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");
> +		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder\n");
>  
>  	hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data);

[Severity: High]
This isn't a bug introduced by this patch, but does dw_hdmi_qp_bind() leak
resources on component unbind?

dw_hdmi_qp_bind() allocates multiple devm_ resources on the component device
such as memory, regmaps, shared IRQs, and bridges. Since the dw_hdmi_qp
library lacks an unbind function, these resources are never freed when the
DRM device unbinds.

On rebind, new duplicate resources will be appended to the component device's
devres list, leading to unbounded memory growth and multiple shared IRQ
handlers firing concurrently.

[Severity: High]
This is a pre-existing issue, but does mixing drmm_ and devm_ allocations in
this component bind callback create a lifecycle mismatch?

Earlier in this function, the hdmi structure is allocated with
drmm_kzalloc(), but devm_request_threaded_irq() is called on the component
dev (which hasn't unbound).

When the user unbinds the main DRM device via sysfs, component unbind
destroys the DRM device and frees the hdmi structure. However, the IRQ
remains registered on the component device. If an interrupt fires, the
handler will access the freed hdmi pointer.

Additionally, dw_hdmi_qp_rockchip_unbind() only calls
cancel_delayed_work_sync(), leaving devm_ resources accumulated on rebind.

>  	if (IS_ERR(hdmi->hdmi))
>  		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi),
> -				     "Failed to bind dw-hdmi-qp");
> +				     "Failed to bind dw-hdmi-qp\n");
>  
>  	connector = drm_bridge_connector_init(drm, encoder);
>  	if (IS_ERR(connector))

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