Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Preexisting double unlock of connection_mutex and lockless access to connector state in drm_bridge_helper_reset_crtc(). -- commit 8efd841f2ea4070344d378811452cf54e3a77546 Author: Cristian Ciocaltea <[email protected]> drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc() This commit removes a no-op error check after drm_atomic_helper_reset_crtc, as the goto target is the immediately following label and the return value is already propagated correctly. > 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; > 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 does this function have a pre-existing double unlock bug due to how drm_atomic_get_connector_for_encoder behaves? Looking at drm_bridge_helper_reset_crtc, it first acquires connection_mutex via drm_modeset_lock. It then calls drm_atomic_get_connector_for_encoder. Because the lock is already held by ctx, the drm_modeset_lock call inside the helper returns 0 rather than an error. However, drm_atomic_get_connector_for_encoder unconditionally calls drm_modeset_unlock on the connection_mutex before returning. Since ww_mutex does not track nesting depths, the lock is completely dropped. Does this lead to connector->state being accessed without the lock just before this hunk, and then a subsequent double unlock when drm_modeset_unlock is called again here at the out label? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=12
