Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [Critical] drm_atomic_get_connector_for_encoder returns a connector pointer without holding a reference, leading to a Use-After-Free. - [High] Double-unlock of connection_mutex due to nested drm_modeset_unlock calls. -- commit d2f773b296aa81ea66367361aef51be8375a8cce Author: Cristian Ciocaltea <[email protected]> drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc() This commit removes a redundant error check after calling drm_atomic_helper_reset_crtc() in drm_bridge_helper_reset_crtc(), as the error path and successful path both immediately fall through to the same unlock label. > diff --git a/drivers/gpu/drm/drm_bridge_helper.c > b/drivers/gpu/drm/drm_bridge_helper.c > index 420f29cf3e543..0a3c8fee66b32 100644 > --- a/drivers/gpu/drm/drm_bridge_helper.c > +++ b/drivers/gpu/drm/drm_bridge_helper.c > @@ -50,8 +50,6 @@ int drm_bridge_helper_reset_crtc(struct drm_bridge *bridge, > > crtc = connector->state->crtc; [Severity: Critical] This is a pre-existing issue, but does drm_atomic_get_connector_for_encoder() return the connector pointer without holding a reference to it? When drm_atomic_get_connector_for_encoder() iterates over connectors, it calls drm_connector_list_iter_end() before returning, which explicitly drops the reference to the iterator's current connector. The pointer returned to this function is then dereferenced here when accessing connector->state->crtc. Could a concurrent connector hot-unplug event (such as an MST display disconnect) trigger a use-after-free before this dereference? > ret = drm_atomic_helper_reset_crtc(crtc, ctx); > - if (ret) > - goto out; > > out: > drm_modeset_unlock(&dev->mode_config.connection_mutex); [Severity: High] This isn't a bug introduced by this patch, but is it possible for this unlock to cause a double-unlock of connection_mutex? drm_bridge_helper_reset_crtc() acquires connection_mutex via its ctx. It then calls drm_atomic_get_connector_for_encoder(), which attempts to acquire the same mutex with the same ctx. This succeeds immediately with -EALREADY, but before returning, drm_atomic_get_connector_for_encoder() unconditionally calls drm_modeset_unlock(). Since the lock is dropped in the helper, won't this call to drm_modeset_unlock() result in a double-unlock, as well as potential data races for the code executing between the two unlocks? > return ret; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=15
