Re: [PATCH v2] drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property

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

Pre-existing issues:
- [High] Use-After-Free in HPD interrupt handler and workqueue due to mismatched devm and drmm lifecycles on bind error paths.
--

--- Patch [1]: [PATCH v2] drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property ---
commit 2bdde0ebdaa05708637a2fd7def9b7106821d2d5
Author: Igor Paunovic <[email protected]>

drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property

This commit attaches the HDMI Colorspace connector property to the RK3588
HDMI QP output, which allows userspace to request BT.2020 colorimetry. This
enables HDR support for every RK3588 HDMI output.

Link: https://lore.kernel.org/all/[email protected]/
Link: https://lore.kernel.org/all/[email protected]/

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 9c4cf68d79d50..35d04680d0113 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
[ ... ]
> @@ -695,6 +695,13 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
>  				     "Failed to init bridge connector\n");
>  
> +	/* Passing 0 selects the full HDMI colorspace set defined by the core. */
> +	ret = drm_mode_create_hdmi_colorspace_property(connector, 0);
> +	if (ret)
> +		return ret;
[Severity: High]
This is a pre-existing issue, but does adding this new error return path
introduce a use-after-free risk with the HPD interrupt handler?

As noted in the commit message, the uncancelled hpd_work queue is a known
issue on bind error paths, but the same problem applies to the IRQ handler.

In dw_hdmi_qp_rockchip_bind(), the hdmi struct is allocated via
drmm_kzalloc(), tying its lifetime to the DRM device:

	hdmi = drmm_kzalloc(drm, sizeof(*hdmi), GFP_KERNEL);

However, the interrupt is registered using devm_request_threaded_irq()
against the component's platform device (pdev->dev):

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

If drm_mode_create_hdmi_colorspace_property() fails, the component bind
returns an error. This causes the master bind to fail and destroy the DRM
device, which frees the hdmi struct.

Because the component device itself remains successfully probed, its devm
resources (including the IRQ handler) are not rolled back. If the shared
IRQ line fires after this bind failure, will the kernel invoke the handler
with the freed hdmi pointer?

> +
> +	drm_connector_attach_colorspace_property(connector);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1