linux-kernel - Re: [PATCH] platform/x86: int3472: Fix double free of GPIO device during unregister

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: <aQCrLVBBOBVCnoJ8@smile.fi.intel.com>
Date: Tue, 28 Oct 2025 13:38:21 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Dan Scally <dan.scally@...asonboard.com>
Cc: Hans de Goede <hansg@...nel.org>, Qiu Wenbo <qiuwenbo@...me.org>,
	Daniel Scally <djrscally@...il.com>,
	Qiu Wenbo <qiuwenbo@...insec.com.cn>,
	platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Sakari Ailus <sakari.ailus@...ux.intel.com>,
	Andy Shevchenko <andy@...nel.org>
Subject: Re: [PATCH] platform/x86: int3472: Fix double free of GPIO device
 during unregister

On Tue, Oct 28, 2025 at 11:09:12AM +0000, Dan Scally wrote:
> On 28/10/2025 10:54, Andy Shevchenko wrote:
> > On Tue, Oct 28, 2025 at 11:38:00AM +0100, Hans de Goede wrote:
> > > On 28-Oct-25 11:02 AM, Andy Shevchenko wrote:
> > > > On Tue, Oct 28, 2025 at 08:55:07AM +0000, Dan Scally wrote:
> > > > > On 24/10/2025 06:05, Qiu Wenbo wrote:

...

> > > > > However the Fixes tag I wonder about; devm_gpiod_get() will also result in a
> > > > > call to gpiod_put() when the module is unloaded; doesn't that mean that the
> > > > > same issue  will occur before that commit?
> > > > 
> > > > Actually a good question! To me sounds like it's a bug(?) in regulator code.
> > > > It must not release resources it didn't acquire. This sounds like a clear
> > > > layering violation.
> > > 
> > > I think the problem is that when it comes from devicetree it is acquired
> > > by the regulator core.
> > 
> > Hmm... I probably missed that, but I failed to see this. Any pointers?
> 
> They can come through the struct regulator_desc.of_parse_cb(), which is called in
> regulator_of_init_data(), from regulator_register(). For example: https://elixir.bootlin.com/linux/v6.17.5/source/drivers/power/supply/mt6370-charger.c#L234>

Ah, thank you, Dan, for the pointers. Indeed, that's how it's done. Hmm, still
why can't we let the regulator consumer to decide when to clean the resource?
I think this is an attempt to have a refcounting against shared GPIO resource
and it should be done in the GPIOLIB (if not yet). In regulator that put
call should probably be conditional (based on the source of GPIO request).

> > > Only when passed as platform-data as we do here does
> > > this layering violation occur.
> > > 
> > > I do believe that a transfer of ownership ad done here is ok for
> > > the platform-data special case.

As an exception, maybe...

-- 
With Best Regards,
Andy Shevchenko