diff mbox series

[v9,10/61] drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc()

Message ID 20260723-dw-hdmi-qp-scramb-v9-10-4fb12ea22ac9@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 23, 2026, 1:34 a.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>
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 23, 2026, 1:47 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional unlocking of a context-managed lock causes lock stripping and double unlocks.
- [High] drm_atomic_get_connector_for_encoder() returns a connector without holding a reference, leading to a Use-After-Free.
--

--- Patch [10]: [PATCH v9 10/61] drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc() ---
commit 9b5cf69b4268daeb09910d2aa6efb179fd6ada06
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc()

This commit removes the no-op error check after
drm_atomic_helper_reset_crtc() since the goto target is the immediately
following label. The return value is already propagated correctly
without it.

> 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: High]
This is a pre-existing issue, but can this dereference cause a Use-After-Free?

Looking at drm_atomic_get_connector_for_encoder() called earlier in this
function, it finds a matching connector and breaks out of the
drm_for_each_connector_iter() loop. It then calls
drm_connector_list_iter_end(), which drops the reference on the found
connector.

The function then returns this connector without acquiring a new reference
via drm_connector_get().

If a concurrent hotplug event (like an MST display unplug) unregisters and
frees the connector before we dereference connector->state here, would it
result in a Use-After-Free?

>  	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 can this unconditional unlock
lead to a double-unlock?

Earlier in this function, drm_atomic_get_connector_for_encoder() is called
and passed the acquire context ctx. Since connection_mutex was already locked
in ctx, the inner function's lock attempt returns -EALREADY (which is
treated as success).

However, at the end of drm_atomic_get_connector_for_encoder(), it
unconditionally calls drm_modeset_unlock(). This unlinks the lock from the
context and fully unlocks the WW mutex, stripping the lock from
drm_bridge_helper_reset_crtc().

When we reach this out label, does calling drm_modeset_unlock() again on the
already unlocked mutex trigger a double-unlock warning or crash?
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);