Re: [PATCH v10 49/69] drm/bridge: inno-hdmi: Advertise HDMI 1.2 capabilities

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 as=
signed, creating a race window where an I2C transfer can trigger a NULL poi=
nter dereference.
- [High] The threaded IRQ is requested before the DRM bridge is fully initi=
alized and attached, causing a NULL pointer dereference if an early interru=
pt 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 =3D DRM_MODE_CONNECTOR_HDMIA;
>  	hdmi->bridge.vendor =3D "Inno";
>  	hdmi->bridge.product =3D "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 =3D HDMI_VERSION_1_2;
> =20
>  	hdmi->bridge.ddc =3D 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 =3D 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 =3D hdmi->i2c;
mutex_lock(&i2c->lock);

Could this dereference the uninitialized hdmi->i2c pointer, causing a crash?

>  	if (IS_ERR(hdmi->bridge.ddc))

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731-dw-hdmi-qp=
[email protected]?part=3D49