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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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