Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] The I2C adapter is registered and exposed before `hdmi->i2c` is assigned, creating a race window where an I2C transfer can trigger a NULL pointer dereference. - [High] The threaded IRQ is requested before the DRM bridge is fully initialized and attached, causing a NULL pointer dereference if an early interrupt fires. -- --- Patch [49]: [PATCH v10 49/69] drm/bridge: inno-hdmi: Advertise HDMI 1.2 capabilities --- commit edc5142ea37d19616540dca30f30fcf265c228eb Author: Cristian Ciocaltea <[email protected]> drm/bridge: inno-hdmi: Advertise HDMI 1.2 capabilities Bridges using DRM_BRIDGE_OP_HDMI are now expected to also provide a supported HDMI version. This is not yet enforced, but will become mandatory once the migration to the new caps-based HDMI connector initialization API is completed. Since inno_hdmi_phy_config.pixelclock does not exceed 165 MHz for any of the chip variants handled by the driver, advertise HDMI 1.2 as the supported version. > diff --git a/drivers/gpu/drm/bridge/inno-hdmi.c > b/drivers/gpu/drm/bridge/inno-hdmi.c > index dd35f5a875d3d..ea3447d4f285b 100644 > --- a/drivers/gpu/drm/bridge/inno-hdmi.c > +++ b/drivers/gpu/drm/bridge/inno-hdmi.c > @@ -1119,6 +1119,7 @@ struct inno_hdmi *inno_hdmi_bind(struct device *dev, > hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA; > hdmi->bridge.vendor = "Inno"; > hdmi->bridge.product = "Inno HDMI"; [Severity: High] This is a pre-existing issue, but is it possible for an early interrupt to cause a NULL pointer dereference here? Earlier in inno_hdmi_bind(), devm_request_threaded_irq() is called, which can enable the IRQ and trigger inno_hdmi_irq() before the bridge is fully initialized. If that happens, inno_hdmi_irq() calls: drm_helper_hpd_irq_event(hdmi->bridge.dev). However, hdmi->bridge.dev is not initialized until drm_bridge_attach() is called later in the inno_hdmi_bind() sequence. Will this lead to drm_helper_hpd_irq_event() dereferencing a NULL pointer? > + hdmi->bridge.supported_hdmi_ver = HDMI_VERSION_1_2; > > hdmi->bridge.ddc = inno_hdmi_i2c_adapter(hdmi); [Severity: High] This isn't a bug introduced by this patch, but does registering the I2C adapter before assigning the internal data structure pointer create a race condition? Inside inno_hdmi_i2c_adapter(), devm_i2c_add_adapter(hdmi->dev, adap) is called before hdmi->i2c = i2c is set. If a concurrent I2C transfer is initiated (for example, from userspace via i2c-dev) immediately after registration, the transfer callback inno_hdmi_i2c_xfer() will execute. When inno_hdmi_i2c_xfer() runs, it does: struct inno_hdmi_i2c *i2c = hdmi->i2c; mutex_lock(&i2c->lock); Could this dereference the uninitialized hdmi->i2c pointer, causing a crash? > if (IS_ERR(hdmi->bridge.ddc)) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=49
