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 <[email protected]> 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? > 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/[email protected]?part=7
