linux-kernel - Re: [PATCH v8 2/5] drm/bridge: get the bridge returned by drm_bridge_chain_get_first

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250624-precise-loon-of-mastery-ebad24@houat>
Date: Tue, 24 Jun 2025 09:05:09 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Liu Ying <victor.liu@....com>
Cc: Luca Ceresoli <luca.ceresoli@...tlin.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>, 
	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>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Marek Vasut <marex@...x.de>, 
	Stefan Agner <stefan@...er.ch>, Shawn Guo <shawnguo@...nel.org>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	Fabio Estevam <festevam@...il.com>, Hui Pu <Hui.Pu@...ealthcare.com>, 
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>, dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v8 2/5] drm/bridge: get the bridge returned by
 drm_bridge_chain_get_first_bridge()

On Tue, Jun 24, 2025 at 10:44:03AM +0800, Liu Ying wrote:
> On 06/23/2025, Luca Ceresoli wrote:
> > On Mon, 23 Jun 2025 10:56:13 +0800
> > Liu Ying <victor.liu@....com> wrote:
> > 
> >> On 06/21/2025, Luca Ceresoli wrote:
> >>> drm_bridge_chain_get_first_bridge() returns a bridge pointer that the
> >>> caller could hold for a long time. Increment the refcount of the returned
> >>> bridge and document it must be put by the caller.  
> >>
> >> To make sure the incremented refcount is decremented once this patch is
> >> applied, does it make sense to squash patch 3, 4 and 5 into this one?
> > 
> > I see there is a trade off here between bisectability and patch
> > readability.
> > 
> > However about bisectability the problem is limited for this series. To
> > get an actual get/put imbalance you'd have to be able to remove the
> > bridge, but removing (part of) the bridge chain is not at all supported
> > right now, and it won't be until after chapter 4 of this work (see
> > cover letter).
> > 
> > However I realize there is an issue if:
> > * patch 2 is applied but patches 3/4/5 are not
> >   (it does not make sense to apply this series partially, but this
> >   might happen when cherry-picking?)
> 
> Yes for cherry-picking and bisecting.
> 
> > * an entire DRM card is removed where
> >   drm_bridge_chain_get_first_bridge() is used by some components
> > 
> > If both happen we'd have a get without put, thus a missing free and a
> > memory leak for the container struct.
> 
> Yes, that's a memory leak.
> 
> > Note that, besides drm_bridge_chain_get_first_bridge() that this
> > series covers, there are various other accessors: see items 1.E.{2..8}
> 
> IIUC, without those items addressed, the issue we have is use-after-free,
> but not the memory leak this patch introduces(without squash).

Given that this structure is going to be allocated a couple of times in
the system life at best, and that the situation prior to the work Luca
has been doing was a use-after-free, I'm not really concerned about a
transient memory leak in a situation that cannot happen.

If people want to come and backport random patches without looking at
the whole thing, that's their problem.

Maxime

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