| 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
| ||
|
Message-ID: <2127da59-c8aa-301e-4648-2abf48ea7d61@gmx.de>
Date: Fri, 26 Aug 2022 20:07:25 +0200
From: Armin Wolf <W_Armin@....de>
To: Guenter Roeck <linux@...ck-us.net>
Cc: jdelvare@...e.com, linux-hwmon@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] hwmon: Add include stubs
Am 26.08.22 um 14:47 schrieb Guenter Roeck:
> On Thu, Aug 25, 2022 at 11:43:40PM +0200, Armin Wolf wrote:
>> If CONFIG_HWMON/CONFIG_HWMON_VID was disabled during compile
>> time, driver using the hwmon subsystem might fail to compile.
> That would be a bug in driver dependencies and is not a problem
> to be solved in the hwmon subsystem. Dummies are not provided on
> purpose so far because we _want_ driver developers to think about
> usage and for them to understand what happens if HWMON is not enabled.
My wording might have been incorrect, i meant that currently, code who uses
the hwmon subsystem must either select CONFIG_HWMON or provide #ifdef guards.
> The benefit is that it would reduce the need for conditional code
> in drivers registering with hwmon from outside the hwmon subsystem.
>
>> Provide stubs for such cases.
>>
> HWMON_VID is not user selectable, it is only needed by hwmon drivers,
> it is mandatory for the drivers needing it, those drivers _must_ select
> it, and there must be no dummies.
>
> I am not really sure about the benefits of dummies for HWMON either,
> but I am open to discussion. Either case that must be accompanied
> by matching driver patches to show its usage, to make sure that there
> is no negative impact, to show the benefits, and to get a wider audience.
>
Understandable, i will include such patches in the next revision.
>> Signed-off-by: Armin Wolf <W_Armin@....de>
>> ---
>> include/linux/hwmon-vid.h | 18 ++++++++++
>> include/linux/hwmon.h | 76 ++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 93 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/hwmon-vid.h b/include/linux/hwmon-vid.h
>> index 9409e1d207ef..329151416c47 100644
>> --- a/include/linux/hwmon-vid.h
>> +++ b/include/linux/hwmon-vid.h
>> @@ -11,9 +11,27 @@
>> #ifndef _LINUX_HWMON_VID_H
>> #define _LINUX_HWMON_VID_H
>>
>> +#include <linux/kconfig.h>
>> +
>> +#if IS_ENABLED(CONFIG_HWMON_VID)
>> +
>> int vid_from_reg(int val, u8 vrm);
>> u8 vid_which_vrm(void);
>>
>> +#else
>> +
>> +static inline int vid_from_reg(int val, u8 vrm)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline u8 vid_which_vrm(void)
>> +{
>> + return 0;
>> +}
>> +
>> +#endif /* CONFIG_HWMON_VID */
>> +
>> /* vrm is the VRM/VRD document version multiplied by 10.
>> val is in mV to avoid floating point in the kernel.
>> Returned value is the 4-, 5- or 6-bit VID code.
>> diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
>> index 14325f93c6b2..281387ee03bc 100644
>> --- a/include/linux/hwmon.h
>> +++ b/include/linux/hwmon.h
>> @@ -13,6 +13,9 @@
>> #define _HWMON_H_
>>
>> #include <linux/bitops.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/kconfig.h>
>>
>> struct device;
>> struct attribute_group;
>> @@ -433,6 +436,8 @@ struct hwmon_chip_info {
>> const struct hwmon_channel_info **info;
>> };
>>
>> +#if IS_ENABLED(CONFIG_HWMON)
> This should be IS_REACHABLE(). It doesn't help if HWMON is built as
> module and called from an in-kernel driver. Otherwise drivers using it
> would still need "depends on HWMON || HWMON=n" and it would still require
> conditional code to catch "HWMON enabled but not reachable".
>
>> +
>> /* hwmon_device_register() is deprecated */
>> struct device *hwmon_device_register(struct device *dev);
>>
>> @@ -467,6 +472,75 @@ int hwmon_notify_event(struct device *dev, enum hwmon_sensor_types type,
>> char *hwmon_sanitize_name(const char *name);
>> char *devm_hwmon_sanitize_name(struct device *dev, const char *name);
>>
>> +#else
>> +
>> +static inline struct device *hwmon_device_register(struct device *dev)
>> +{
>> + return ERR_PTR(-ENODEV);
> -ENOTSUPP would probably be a more suitable error code.
The stubs found in other header files do return -ENODEV, so i though
that this should also be done inside hwmon.h
Armin Wolf
>> +}
>> +
>> +static inline struct device
>> +*hwmon_device_register_with_groups(struct device *dev, const char *name, void *drvdata,
>> + const struct attribute_group **groups)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static inline struct device
>> +*devm_hwmon_device_register_with_groups(struct device *dev, const char *name, void *drvdata,
>> + const struct attribute_group **groups)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static inline struct device
>> +*hwmon_device_register_with_info(struct device *dev, const char *name, void *drvdata,
>> + const struct hwmon_chip_info *info,
>> + const struct attribute_group **extra_groups)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static inline struct device *hwmon_device_register_for_thermal(struct device *dev, const char *name,
>> + void *drvdata)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static inline struct device
>> +*devm_hwmon_device_register_with_info(struct device *dev, const char *name, void *drvdata,
>> + const struct hwmon_chip_info *info,
>> + const struct attribute_group **extra_groups)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static inline void hwmon_device_unregister(struct device *dev)
>> +{
>> +}
>> +
>> +static inline void devm_hwmon_device_unregister(struct device *dev)
>> +{
>> +}
>> +
>> +static inline int hwmon_notify_event(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>> + int channel)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> +static inline char *hwmon_sanitize_name(const char *name)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static inline char *devm_hwmon_sanitize_name(struct device *dev, const char *name)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>> +
>> +#endif /* CONFIG_HWMON */
>> +
>> /**
>> * hwmon_is_bad_char - Is the char invalid in a hwmon name
>> * @ch: the char to be considered
>> @@ -490,4 +564,4 @@ static inline bool hwmon_is_bad_char(const char ch)
>> }
>> }
>>
>> -#endif
>> +#endif /* _HWMON_H_ */
>> --
>> 2.30.2
>>
Powered by blists - more mailing lists