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: <ca09333e-902e-4b2c-8c8c-eb7f0d2d4922@roeck-us.net>
Date: Thu, 12 Sep 2024 07:13:25 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Matthew Sanders <m@...ande.rs>, jdelvare@...e.com,
 linux-hwmon@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, ppwaskie@...nel.org
Subject: Re: [PATCH] hwmon: Fix WARN_ON() always called from
 devm_hwmon_device_unregister()

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.

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);