Re: [PATCH v10 14/69] drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double unlock of connection_mutex and unprotected state access due to nested lock handling flaw in drm_atomic_get_connector_for_encoder.
--

--- Patch [14]: [PATCH v10 14/69] drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc() ---
commit 3469d1c48eb06940af7b278f9266400f3b5ea3dc
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 return value is already propagated correctly by the fall-through to
the out 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;
>  	ret = drm_atomic_helper_reset_crtc(crtc, ctx);
> -	if (ret)
> -		goto out;
>  
>  out:
>  	drm_modeset_unlock(&dev->mode_config.connection_mutex);

[Severity: High]
This is a pre-existing issue, but does this result in a double-unlock of
connection_mutex?

Earlier in this function, the lock is acquired before calling
drm_atomic_get_connector_for_encoder():

drm_bridge_helper_reset_crtc() {
    ...
    ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
    if (ret)
        return ret;

    connector = drm_atomic_get_connector_for_encoder(encoder, ctx);
    ...
}

However, drm_atomic_get_connector_for_encoder() unconditionally unlocks
the mutex on exit. Because drm_modeset_unlock() lacks a nesting counter, it
fully drops the ww_mutex.

Consequently, connector->state is accessed locklessly, and reaching this out
label calls drm_modeset_unlock() a second time, which could corrupt the
kernel lock state.

>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14