Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing synchronization lock between DRM atomic disable and ALSA callbacks leaves a Time-Of-Check to Time-Of-Use (TOCTOU) race condition, failing to fix the kernel panic. -- commit 41e47d6bcdba49646deba6ac74845c934529f2f4 Author: Frank Zhang <[email protected]> drm/bridge: dw-hdmi-qp: Guard clear_audio_infoframe when PHY is down This commit attempts to fix a kernel panic caused when an audio server closes the HDMI audio device concurrently with an atomic_disable. It tries to guard register accesses in the audio teardown path by checking the tmds_char_rate state variable. > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c > index cfd1b18c06e62..f0e9324249d66 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c [ ... ] > @@ -893,6 +893,16 @@ static int > dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge) > return 0; > } > > +static int dw_hdmi_qp_bridge_clear_audio_infoframe(struct drm_bridge *bridge) > +{ > + struct dw_hdmi_qp *hdmi = bridge->driver_private; > + > + if (hdmi->tmds_char_rate > 0) > + return __dw_hdmi_qp_bridge_clear_audio_infoframe(bridge); > + > + return 0; > +} > + [Severity: High] This isn't a bug introduced by this patch, but does this lockless check leave an open Time-Of-Check to Time-Of-Use (TOCTOU) race condition? If a userspace audio server (like pipewire) closes the HDMI audio device at the same time as a DRM modeset disables the display, the ALSA teardown path can execute dw_hdmi_qp_bridge_clear_audio_infoframe() and successfully evaluate this unguarded check. Concurrently, dw_hdmi_qp_bridge_atomic_disable() can execute: hdmi->tmds_char_rate = 0; hdmi->phy.ops->disable(hdmi, hdmi->phy.data); This zeroes the state variable and powers off the PHY without synchronization. The ALSA thread can then resume and invoke __dw_hdmi_qp_bridge_clear_audio_infoframe(), writing to the now-disabled PHY registers and triggering the Asynchronous SError Interrupt panic this patch is trying to fix. Additionally, could the same race exist in dw_hdmi_qp_audio_prepare()? if (!hdmi->tmds_char_rate) return -ENODEV; // ... further down ... dw_hdmi_qp_set_audio_interface(hdmi, fmt, hparms); This path configures multiple registers after a lockless check, making the entire preparation sequence vulnerable to the same race condition. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
