diff mbox series

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

Message ID 20260731-dw-hdmi-qp-scramb-v10-14-294364b2cf15@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 31, 2026, 4:19 p.m. UTC 
Remove the no-op error check after drm_atomic_helper_reset_crtc() since
the goto target is the immediately following label and the return value
is already propagated correctly without it.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Tested-by: Diederik de Haas <diederik@cknow-tech.com>  # NanoPC-T6 LTS, Rock 5B
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/drm_bridge_helper.c | 2 --
 1 file changed, 2 deletions(-)

Comments

sashiko-bot@kernel.org July 31, 2026, 6:01 p.m. UTC | #1 
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 <cristian.ciocaltea@collabora.com>

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;
>  }
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_bridge_helper.c b/drivers/gpu/drm/drm_bridge_helper.c
index 420f29cf3e54..0a3c8fee66b3 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);