Thread (53 messages) flat view 53 messages, 6 authors, 1d ago

Re: [PATCH v4 07/20] drm/bridge: inno-hdmi: Split probe out of bind

From: sashiko-bot@kernel.org
Date: 2026-09-15 15:44:25
Also in: dri-devel, linux-clk, linux-devicetree

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] drm/bridge: inno-hdmi: Meaningful code and commit message mismatch for inno_hdmi_remove()
- [Medium] drm/bridge: inno-hdmi: TOCTOU race on hdmi->bridge.dev leading to potential NULL pointer dereference
--

commit 619d70bf74f8f0e498489e8aa8a7c0c9e4e2d3fb
Author: Michal Wilczynski [off-list ref]

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

This commit splits the bridge setup into a new exported inno_hdmi_probe()
function and reduces inno_hdmi_bind() to a wrapper around it. This
decoupling allows a platform device to register its own bridge without
attaching it to the DRM encoder immediately.
    Move the setup into a new exported inno_hdmi_probe(), with a matching
    inno_hdmi_remove(), and reduce inno_hdmi_bind() to a wrapper around it.
[Severity: Medium]
Is the commit message description accurate regarding the removal function?

The commit message explicitly claims to introduce a matching
inno_hdmi_remove() function for teardown alongside inno_hdmi_probe(), but
this function appears to be missing from the patch. Does this break the
expected API contract described for external decoupled drivers?
quoted hunk ↗ jump to hunk
diff --git a/drivers/gpu/drm/bridge/inno-hdmi.c b/drivers/gpu/drm/bridge/inno-hdmi.c
index dd35f5a875d3d..840f1c7bc2029 100644
--- a/drivers/gpu/drm/bridge/inno-hdmi.c
+++ b/drivers/gpu/drm/bridge/inno-hdmi.c
@@ -929,7 +929,14 @@ static irqreturn_t inno_hdmi_irq(int irq, void *dev_id)
 {
 	struct inno_hdmi *hdmi = dev_id;
 
-	drm_helper_hpd_irq_event(hdmi->bridge.dev);
+	/*
+	 * The interrupt is requested in probe, but bridge.dev is only set once
+	 * the DRM master binds and attaches the bridge, which may never happen.
+	 * Drop hotplug events that arrive before then rather than dereference a
+	 * NULL drm_device.
+	 */
+	if (hdmi->bridge.dev)
+		drm_helper_hpd_irq_event(hdmi->bridge.dev);
[Severity: Medium]
Can this lockless check of hdmi->bridge.dev lead to a NULL pointer
dereference?

Without READ_ONCE(), the compiler might double-fetch the pointer. If the
pointer is cleared concurrently (for example, during a failed
drm_bridge_attach() or a teardown path) after the check but before it is
passed to drm_helper_hpd_irq_event(), it could result in a NULL pointer
dereference.

Could we use READ_ONCE() and a local variable to prevent a potential race
condition in inno_hdmi_irq()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-jh7110-clean-send-v4-0-f0e4fd6f2cc8@samsung.com?part=7

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help