Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Memory leak and global list corruption (dangling pointer) if component_add() fails in sti_hdmi_probe(). - [High] Resource leak of the audio platform device and potential NULL pointer dereference if sti_hdmi_bind() fails. -- --- Patch [36]: [PATCH v2 36/37] drm/sti: hdmi: Switch to atomic bridge callbacks --- commit fe4cd2364e238c4d447b17cda3c258ebb7c65628 Author: Maxime Ripard <[email protected]> drm/sti: hdmi: Switch to atomic bridge callbacks The hdmi bridge uses the legacy non-atomic pre_enable, enable, disable and post_disable bridge callbacks. In order to remove the legacy bridge callback support from the DRM bridge core, switch to their atomic counterparts and add the bridge atomic state handlers. > diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c > index f8222e60b1e01..7ba9e0b7ba6c0 100644 > --- a/drivers/gpu/drm/sti/sti_hdmi.c > +++ b/drivers/gpu/drm/sti/sti_hdmi.c [ ... ] > @@ -964,16 +966,20 @@ static void sti_hdmi_set_mode(struct drm_bridge *bridge, > } > } > > -static void sti_hdmi_bridge_nope(struct drm_bridge *bridge) > +static void sti_hdmi_bridge_nope(struct drm_bridge *bridge, > + struct drm_atomic_commit *commit) > { > /* do nothing */ > } > > static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = { > - .pre_enable = sti_hdmi_pre_enable, > - .enable = sti_hdmi_bridge_nope, > - .disable = sti_hdmi_disable, > - .post_disable = sti_hdmi_bridge_nope, > + .atomic_create_state = drm_atomic_helper_bridge_create_state, > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > + .atomic_pre_enable = sti_hdmi_pre_enable, > + .atomic_enable = sti_hdmi_bridge_nope, > + .atomic_disable = sti_hdmi_disable, > + .atomic_post_disable = sti_hdmi_bridge_nope, > .mode_set = sti_hdmi_set_mode, > }; [Severity: High] This is a pre-existing issue, but does sti_hdmi_probe() leak memory and leave a dangling pointer if component_add() fails? drivers/gpu/drm/sti/sti_hdmi.c:sti_hdmi_probe() { ... drm_bridge_add(&hdmi->bridge); return component_add(&pdev->dev, &sti_hdmi_ops); } If component_add() fails, the function returns an error without calling drm_bridge_remove(). Because the bridge is never removed, the reference count does not drop to zero during devres cleanup. Could another driver try to access the corrupted bridge in the global bridge_list, leading to a use-after-free panic when it accesses the freed devres-managed resources? [Severity: High] This isn't a bug introduced by this patch, but does the error handling in sti_hdmi_bind() leave the audio platform device registered while setting the connector to NULL? drivers/gpu/drm/sti/sti_hdmi.c:sti_hdmi_bind() { ... err_sysfs: hdmi->drm_connector = NULL; return -EINVAL; } If sti_hdmi_register_audio_driver() succeeds but a subsequent initialization step fails, we jump to the err_sysfs label, which clears hdmi->drm_connector without unregistering the active audio device. Since the ALSA framework may still invoke operations on the registered codec, can this trigger a NULL pointer dereference when hdmi_audio_get_eld() is called? drivers/gpu/drm/sti/sti_hdmi.c:hdmi_audio_get_eld() { ... struct drm_connector *connector = hdmi->drm_connector; DRM_DEBUG_DRIVER("\n"); mutex_lock(&connector->eld_mutex); ... } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=36
