Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
to consider:
- [High] drm/bridge: premature bridge registration causes UAF on probe failure
- [High] drm/bridge: missing HDMI formats and max_bpc initialization causes 
display failure
--

commit 845a14001f40c5c49616eeff3efbd5f04313cc72
Author: Sunyun Yang <[email protected]>

drm/bridge: Add Lontium LT9611C(EX/UXD) MIPI DSI to HDMI driver

This commit introduces the DRM bridge driver for the Lontium
LT9611C(EX/UXD) MIPI DSI to HDMI converter chips. It supports single or
dual MIPI DSI and I2S signals to HDMI output.

> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611c.c 
> b/drivers/gpu/drm/bridge/lontium-lt9611c.c
> new file mode 100644
> index 0000000000000..54ae535339f74
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611c.c
[ ... ]
> @@ -1160,20 +1160,20 @@
[ ... ]
> +     lt9611c->bridge.of_node = client->dev.of_node;
> +     lt9611c->bridge.ops = DRM_BRIDGE_OP_DETECT |
> +                     DRM_BRIDGE_OP_EDID |
> +                     DRM_BRIDGE_OP_HPD |
> +                     DRM_BRIDGE_OP_HDMI |
> +                     DRM_BRIDGE_OP_HDMI_AUDIO;

[Severity: High]
Will setting the DRM_BRIDGE_OP_HDMI flag here cause connector initialization
to fail?

It looks like the bridge.supported_formats and bridge.max_bpc fields are not
initialized before this is set. When drm_bridge_connector_init() calls the
core helper drm_connector_hdmi_init(), it strictly validates these parameters.
Because they are implicitly zero-initialized, drm_connector_hdmi_init() will
return -EINVAL, which would deterministically fail the connector creation and
render the display driver non-functional.

Should these HDMI capabilities be explicitly set before enabling the HDMI ops
flag?

> +     lt9611c->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
> +
> +     lt9611c->bridge.vendor = "Lontium";
> +     lt9611c->bridge.product = "LT9611C";
> +
> +     lt9611c->bridge.hdmi_audio_dev = dev;
> +     lt9611c->bridge.hdmi_audio_max_i2s_playback_channels = 8;
> +     lt9611c->bridge.hdmi_audio_dai_port = 2;
> +
> +     drm_bridge_add(&lt9611c->bridge);
> +
> +     /* Attach primary DSI */
> +     dsi = lt9611c_attach_dsi(lt9611c, dsi0_node);
> +     if (IS_ERR(dsi)) {
> +             ret = PTR_ERR(dsi);
> +             goto err_remove_bridge;
> +     }

[Severity: High]
Can this sequence lead to a use-after-free if the DSI attachment fails?

Because drm_bridge_add() makes the bridge globally visible on the bridge_list,
another KMS driver could concurrently discover and attach to it during this
window. If the subsequent lt9611c_attach_dsi() call fails (for example,
returning -EPROBE_DEFER), the driver calls drm_bridge_remove() and returns an
error, which triggers devres to free the memory backing the bridge.

Could the concurrent KMS driver be left holding a dangling pointer to
devres-freed memory? Should drm_bridge_add() be deferred until all resources
are successfully acquired?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to