linux-kernel - Re: [PATCH 8/9] drm/bridge: put the bridge returned by drm_bridge_get_next

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250710-classic-bouncy-caiman-8e2045@houat>
Date: Thu, 10 Jul 2025 09:27:09 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc: 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>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Liu Ying <victor.liu@....com>, Shawn Guo <shawnguo@...nel.org>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	Fabio Estevam <festevam@...il.com>, Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, 
	Philipp Zabel <p.zabel@...gutronix.de>, Hui Pu <Hui.Pu@...ealthcare.com>, 
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>, linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 8/9] drm/bridge: put the bridge returned by
 drm_bridge_get_next_bridge()

Hi,

On Wed, Jul 09, 2025 at 06:48:07PM +0200, Luca Ceresoli wrote:
> The bridge returned by drm_bridge_get_next_bridge() is refcounted. Put it
> when done.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>

You should really expand a bit more your commit logs, and provide the
context of why you think putting drm_bridge_put where you do is a good idea.

> ---
>  drivers/gpu/drm/drm_bridge.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0b450b334afd82e0460f18fdd248f79d0a2b153d..05e85457099ab1e0a23ea7842c9654c9a6881dfb 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -1147,6 +1147,8 @@ drm_atomic_bridge_propagate_bus_flags(struct drm_bridge *bridge,
>  	} else {
>  		next_bridge_state = drm_atomic_get_new_bridge_state(state,
>  								next_bridge);
> +		drm_bridge_put(next_bridge);
> +
>  		/*
>  		 * No bridge state attached to the next bridge, just leave the
>  		 * flags to 0.

In particular, I don't think it is here.

You still have a variable in scope after that branch that you would have
given up the reference for, which is pretty dangerous.

Also, the bridge state lifetime is shorter than the bridge lifetime
itself, so we probably want to have the drm_bridge_put after we're done
with next_bridge_state too.

Overall, I think using __free here is probably the most robust solution.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)