Re: [PATCH v7 08/19] drm/bridge: inno-hdmi: Split probe out of bind

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/bridge: inno-hdmi: race on bridge.dev in inno_hdmi_irq()
  after probe split
--

commit b0209990726be89f8d7446a146a593a4fe328e8b
Author: Michal Wilczynski <m.wilczynski@samsung.com>

drm/bridge: inno-hdmi: Split probe out of bind

This commit splits hardware and bridge setup out of inno_hdmi_bind() into
an exported inno_hdmi_probe() helper so standalone platform drivers can
register the bridge without immediately attaching it to an encoder.

> diff --git a/drivers/gpu/drm/bridge/inno-hdmi.c b/drivers/gpu/drm/bridge/inno-hdmi.c
> index 5bfe59f18823a..aa64ffdbd3072 100644
> --- a/drivers/gpu/drm/bridge/inno-hdmi.c
> +++ b/drivers/gpu/drm/bridge/inno-hdmi.c
> @@ -1063,11 +1063,24 @@ static struct i2c_adapter *inno_hdmi_i2c_adapter(struct inno_hdmi *hdmi)
>  	return adap;
>  }
>  
> -struct inno_hdmi *inno_hdmi_bind(struct device *dev,
> -				 struct drm_encoder *encoder,
> -				 const struct inno_hdmi_plat_data *plat_data)
> +/**
> + * inno_hdmi_probe - Internal helper to perform common setup
> + * @pdev: platform device
> + * @plat_data: SoC-specific platform data
> + *
> + * This function handles all the common hardware setup: allocating the main
> + * struct, mapping registers, getting clocks, initializing the hardware,
> + * setting up the IRQ, and initializing the DDC adapter and bridge struct.
> + *
> + * It registers the bridge but does not attach it, for glue drivers that probe
> + * as their own platform device rather than through a component bind.
> + *
> + * Return: the inno_hdmi instance on success, or an ERR_PTR() on failure.
> + */
> +struct inno_hdmi *inno_hdmi_probe(struct platform_device *pdev,
> +				  const struct inno_hdmi_plat_data *plat_data)
>  {
> -	struct platform_device *pdev = to_platform_device(dev);
> +	struct device *dev = &pdev->dev;
>  	struct inno_hdmi *hdmi;
>  	int irq;
>  	int ret;
> @@ -1131,7 +1144,23 @@ struct inno_hdmi *inno_hdmi_bind(struct device *dev,
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> -	ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> +	return hdmi;
> +}
> +EXPORT_SYMBOL_GPL(inno_hdmi_probe);

[Severity: Medium]
Can inno_hdmi_irq() race with drm_bridge_detach() on hdmi->bridge.dev when
a standalone bridge driver uses inno_hdmi_probe()?

Previously, inno_hdmi_bind() was called from component bind, so
component_unbind_all() released the devm_request_threaded_irq() handler
before drm_dev_put() detached the bridge and freed the drm_device.  When a
standalone platform driver calls inno_hdmi_probe(), the threaded IRQ stays
active for the lifetime of the bridge platform_device while the display
driver attaches and detaches the bridge.

drivers/gpu/drm/bridge/inno-hdmi.c:inno_hdmi_irq() {
	/* bridge.dev is NULL until drm_bridge_attach() */
	if (hdmi->bridge.dev)
		drm_helper_hpd_irq_event(hdmi->bridge.dev);
}

If an HPD interrupt fires while the display driver is tearing down its
drm_device, inno_hdmi_irq() reads hdmi->bridge.dev without synchronization
against drm_bridge_detach() and drm_dev_put():

CPU0                                  CPU1 (irq/inno_hdmi_irq)
drm_dev_put(drm)
  ...
    drm_encoder_cleanup(encoder)
      drm_bridge_detach(&hdmi->bridge)
                                      if (hdmi->bridge.dev)
        bridge->dev = NULL;
  kfree(drm)
                                        drm_helper_hpd_irq_event(...)
                                          <-- use-after-free or NULL deref

Depending on timing, this causes either a use-after-free of the drm_device
or a NULL pointer dereference in drm_helper_hpd_irq_event() if the compiler
reloads hdmi->bridge.dev.  Calling drm_bridge_hpd_notify(&hdmi->bridge,
status) instead of drm_helper_hpd_irq_event() would serialize against HPD
disable through bridge->hpd_mutex.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-jh7110-clean-send-v7-0-68f7ea9913db@samsung.com?part=8