linux-kernel - Re: [PATCH] hwmon: Fix WARN_ON() always called from devm_hwmon_device

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: <488b3bdf870ea76c4b943dbe5fd15ac8113019dc.camel@kernel.org>
Date: Thu, 12 Sep 2024 23:40:08 -0700
From: PJ Waskiewicz <ppwaskie@...nel.org>
To: Guenter Roeck <linux@...ck-us.net>, Matthew Sanders <m@...ande.rs>, 
	jdelvare@...e.com, linux-hwmon@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: Fix WARN_ON() always called from
 devm_hwmon_device_unregister()

On Thu, 2024-09-12 at 07:13 -0700, Guenter Roeck wrote:
> On 9/12/24 02:14, Matthew Sanders wrote:
> > devm_hwmon_device_unregister() only takes parent of a devres-
> > managed
> > hwmon device as an argument. This always fails, as devres can't
> > find
> > the hwmon device it needs to unregister with the parent device
> > alone.
> > Without this patch, the WARN_ON() in devm_hwmon_device_unregister()
> > will
> > always be displayed unconditionally:
> > 
> > [    7.969746] WARNING: CPU: 1 PID: 224 at
> > drivers/hwmon/hwmon.c:1205 devm_hwmon_device_unregister+0x28/0x30
> > 
> > This patch adds an extra argument to
> > devm_hwmon_device_unregister(), a
> > pointer to a hwmon device which was previously registered to the
> > parent using devres.
> > 
> > There aren't any drivers which currently make use of this function,
> > so
> > any existing users of devm_hwmon_* shouldn't require any changes as
> > a
> > result of this patch.
> 
> If there are no users, there is no need to keep the function. We
> should drop
> it instead.

There aren't any direct in-tree users of the function.  But some out-
of-tree drivers can find it useful to use, hence Matt hitting this
issue.

If there's a desire to just remove it, that's fine.  But it would
remove a handy hook for out-of-tree stuff.

-PJ

> 
> Also, your patch is not signed and therefore can not be applied.
> 
> Guenter
> 
> > ---
> >   drivers/hwmon/hwmon.c | 6 ++++--
> >   include/linux/hwmon.h | 2 +-
> >   2 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> > index a362080d41fa..84945a276320 100644
> > --- a/drivers/hwmon/hwmon.c
> > +++ b/drivers/hwmon/hwmon.c
> > @@ -1199,10 +1199,12 @@ static int devm_hwmon_match(struct device
> > *dev, void *res, void *data)
> >    * devm_hwmon_device_unregister - removes a previously registered
> > hwmon device
> >    *
> >    * @dev: the parent device of the device to unregister
> > + * @hwdev: the hwmon device to unregister
> >    */
> > -void devm_hwmon_device_unregister(struct device *dev)
> > +void devm_hwmon_device_unregister(struct device *dev, struct
> > device *hwdev)
> >   {
> > -	WARN_ON(devres_release(dev, devm_hwmon_release,
> > devm_hwmon_match, dev));
> > +	WARN_ON(devres_release(dev, devm_hwmon_release,
> > devm_hwmon_match,
> > +			       hwdev));
> >   }
> >   EXPORT_SYMBOL_GPL(devm_hwmon_device_unregister);
> >   
> > diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
> > index e94314760aab..2434c6fc17a7 100644
> > --- a/include/linux/hwmon.h
> > +++ b/include/linux/hwmon.h
> > @@ -481,7 +481,7 @@ devm_hwmon_device_register_with_info(struct
> > device *dev,
> >   				const struct attribute_group
> > **extra_groups);
> >   
> >   void hwmon_device_unregister(struct device *dev);
> > -void devm_hwmon_device_unregister(struct device *dev);
> > +void devm_hwmon_device_unregister(struct device *dev, struct
> > device *hwdev);
> >   
> >   int hwmon_notify_event(struct device *dev, enum
> > hwmon_sensor_types type,
> >   		       u32 attr, int channel);
>