linux-kernel - Re: [PATCH 2/7] drm/encoder: drm_encoder_cleanup: take chain mutex while tearing down

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251003123726.4bf38c76@booty>
Date: Fri, 3 Oct 2025 12:37:26 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Maxime Ripard <mripard@...nel.org>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann
 <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter
 <simona@...ll.ch>, Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong
 <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, Laurent
 Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman
 <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, Hui Pu
 <Hui.Pu@...ealthcare.com>, Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/7] drm/encoder: drm_encoder_cleanup: take chain mutex
 while tearing down

Hello,

On Mon, 29 Sep 2025 16:31:27 +0200
Luca Ceresoli <luca.ceresoli@...tlin.com> wrote:

> > > --- a/drivers/gpu/drm/drm_encoder.c
> > > +++ b/drivers/gpu/drm/drm_encoder.c
> > > @@ -195,9 +195,11 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> > >  	 * the indices on the drm_encoder after us in the encoder_list.
> > >  	 */
> > >  
> > > +	mutex_lock(&encoder->bridge_chain_mutex);
> > >  	list_for_each_entry_safe(bridge, next, &encoder->bridge_chain,
> > >  				 chain_node)
> > >  		drm_bridge_detach(bridge);
> > > +	mutex_unlock(&encoder->bridge_chain_mutex);    
> > 
> > You were claiming that the mutex was to prevent issues with concurrent
> > iteration and removal of the list members. list_for_each_entry_safe() is
> > explicitly made to protect against that. Why do we need both?  
> 
> You're right saying we don't need both. With a mutex preventing the list
> from any change, we can actually simpify code a bit to use the non-safe
> list macro:
> 
> -	struct drm_bridge *bridge, *next;
> +	struct drm_bridge *bridge;
> ...
> +	mutex_lock(&encoder->bridge_chain_mutex);
> - 	list_for_each_entry_safe(bridge, next, &encoder->bridge_chain,
> + 	list_for_each_entry(bridge, &encoder->bridge_chain,
>  				 chain_node)
>  		drm_bridge_detach(bridge);
> +	mutex_unlock(&encoder->bridge_chain_mutex);

After looking at it better I realized the _safe variant here is still
needed as the current loop entry is removed inside the loop. The
non-safe version, at the end of the first iteration, would look for the
next element in the now-removed list_head, thus being derailed.

v2 on its way with this taken into account along with the other
discussed items.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com