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 [off-list ref]
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.
quoted hunk ↗ jump to hunk
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