Re: [PATCH 13/24] drm/msm/hdmi: use dev_get_drvdata() in msm_hdmi_unbind()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free of `hdmi` and `hdmi->i2c` due to missing IRQ teardown in `msm_hdmi_unbind`.
--

--- Patch [13]: [PATCH 13/24] drm/msm/hdmi: use dev_get_drvdata() in msm_hdmi_unbind() ---
commit fd38b018e0f710b01405e26519af9581c27b6208
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/msm/hdmi: use dev_get_drvdata() in msm_hdmi_unbind()

This commit updates msm_hdmi_unbind() to fetch the hdmi structure directly
using dev_get_drvdata(dev) rather than indirectly through the private kms
pointer. It also removes a NULL check, as the hdmi pointer is guaranteed to
be valid at unbind time.

> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 506a9856459e1..0327098287d14 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -324,11 +324,10 @@ static void msm_hdmi_unbind(struct device *dev, struct device *master,
>  		void *data)
>  {
>  	struct msm_drm_private *priv = dev_get_drvdata(master);
> +	struct hdmi *hdmi = dev_get_drvdata(dev);
>  
> -	if (priv->kms->hdmi) {
> -		msm_hdmi_destroy(priv->kms->hdmi);

[Severity: High]
This is a pre-existing issue, but does this unbind sequence leave an active
IRQ handler that can cause a use-after-free?

In msm_hdmi_modeset_init(), the IRQ is requested using devm_request_irq()
tied to the DRM master device (dev->dev), not the HDMI component device:

msm_hdmi_modeset_init() {
    ...
    ret = devm_request_irq(dev->dev, hdmi->irq,
                msm_hdmi_irq, IRQF_TRIGGER_HIGH,
                "hdmi_isr", hdmi);
    ...
}

When the HDMI platform device is unbound, msm_hdmi_unbind() is called, which
in turn calls msm_hdmi_destroy(hdmi) here and frees hdmi->i2c. Later, the
HDMI device's devres runs and frees the hdmi structure itself. 

Because the DRM master device is not removed during component unbind, its
devres cleanup does not run, and the IRQ remains registered and active.

If a pending or spurious interrupt triggers msm_hdmi_irq() after this point,
will it dereference the freed hdmi structure and call msm_hdmi_i2c_irq()
with the freed hdmi->i2c pointer?

> -		priv->kms->hdmi = NULL;
> -	}
> +	msm_hdmi_destroy(hdmi);
> +	priv->kms->hdmi = NULL;
>  }
>  
>  static const struct component_ops msm_hdmi_ops = {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722-drm-msm-display-interface-v1-0-368c10fe62fd@oss.qualcomm.com?part=13