linux-kernel - Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: protect device resources on unplug

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: <20250820131302.6a2da5ef@booty>
Date: Wed, 20 Aug 2025 13:13:02 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Maxime Ripard <mripard@...nel.org>
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>, Hui Pu <Hui.Pu@...ealthcare.com>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, Dmitry Baryshkov <lumag@...nel.org>
Subject: Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: protect device resources
 on unplug

Hello Maxime,

On Tue, 19 Aug 2025 14:29:32 +0200
Maxime Ripard <mripard@...nel.org> wrote:

> > @@ -1005,7 +1041,24 @@ static void sn65dsi83_remove(struct i2c_client *client)
> >  {
> >  	struct sn65dsi83 *ctx = i2c_get_clientdata(client);
> >  
> > +	drm_bridge_unplug(&ctx->bridge);
> >  	drm_bridge_remove(&ctx->bridge);  
> 
> Shouldn't we merge drm_bridge_unplug with the release part of
> devm_drm_bridge_alloc?

I'm not sure I got what you are suggesting here, sorry.

Do you mean that __devm_drm_bridge_alloc() should add a devres action
to call drm_bridge_unplug(), so the unplug is called implicitly and
does not need to be called explicitly by all drivers?

If that's what you mean, I don't think that would work. Unless I'm
missing something, devres actions are always invoked just after the
driver .remove callback. But we need to call drm_bridge_unplug() at the
beginning (or just before) .remove, at least for drivers that need to do
something in .remove that cannot be done by devm.

In pseudocode:

mybridge_remove()
{
  drm_bridge_unplug(); <-- explicit call as in my patch
  xyz_disable();
  drm_bridge_unplug(); <-- implicitly done by devres
}

We want xyz_disable() to be done after drm_bridge_unplug(), so other
code paths using drm_bridge_enter/exit() won't mess with xyz.

devres actions cannot be added to be executed _before_ .remove, AFAIK.

> > +	/*
> > +	 * sn65dsi83_atomic_disable() should release some resources, but it
> > +	 * cannot if we call drm_bridge_unplug() before it can
> > +	 * drm_bridge_enter(). If that happens, let's release those
> > +	 * resources now.
> > +	 */
> > +	if (ctx->disable_resources_needed) {
> > +		if (!ctx->irq)
> > +			sn65dsi83_monitor_stop(ctx);
> > +
> > +		gpiod_set_value_cansleep(ctx->enable_gpio, 0);
> > +		usleep_range(10000, 11000);
> > +
> > +		regulator_disable(ctx->vcc);
> > +	}  
> 
> I'm not sure you need this. Wouldn't registering a devm action do the
> same thing?

Good idea, thanks. I'll give it a try.

Luca

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