linux-kernel - Re: [PATCH] hwmon: (asus-ec-sensors) read sensors as signed ints

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]

Date:   Sat, 5 Feb 2022 10:01:20 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Eugene Shalygin <eugene.shalygin@...il.com>
Cc:     Oleksandr Natalenko <oleksandr@...alenko.name>,
        Denis Pauk <pauk.denis@...il.com>,
        Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: (asus-ec-sensors) read sensors as signed ints

On Fri, Feb 04, 2022 at 05:30:45PM +0100, Eugene Shalygin wrote:
> Temperature sensor readings are signed, which is hinted by their blank
> value (oxd8, 216 as unsigned and -40 as signed). T_Sensor, Crosshair
> VIII Hero, and a freezer were used to confirm that.
> 
> Here we read fan sensors as signed too, because with their typical
> values and 2-byte width, I can't tell a difference between signed and
> unsigned, as I don't have a high speed chipset fan.
> 
Hmm, I hope we won't get a situation where an unsigned is reported
and needed, but I guess we can deal with that if/when it happens.

> Signed-off-by: Eugene Shalygin <eugene.shalygin@...il.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/asus-ec-sensors.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> index 05244209c0c6..8a04c76527a4 100644
> --- a/drivers/hwmon/asus-ec-sensors.c
> +++ b/drivers/hwmon/asus-ec-sensors.c
> @@ -221,7 +221,7 @@ static const struct dmi_system_id asus_ec_dmi_table[] __initconst = {
>  
>  struct ec_sensor {
>  	unsigned int info_index;
> -	u32 cached_value;
> +	s32 cached_value;
>  };
>  
>  struct ec_sensors_data {
> @@ -408,15 +408,15 @@ static int asus_ec_block_read(const struct device *dev,
>  	return status;
>  }
>  
> -static inline u32 get_sensor_value(const struct ec_sensor_info *si, u8 *data)
> +static inline s32 get_sensor_value(const struct ec_sensor_info *si, u8 *data)
>  {
>  	switch (si->addr.components.size) {
>  	case 1:
> -		return *data;
> +		return (s8)*data;
>  	case 2:
> -		return get_unaligned_be16(data);
> +		return (s16)get_unaligned_be16(data);
>  	case 4:
> -		return get_unaligned_be32(data);
> +		return (s32)get_unaligned_be32(data);
>  	default:
>  		return 0;
>  	}
> @@ -462,7 +462,7 @@ static int update_ec_sensors(const struct device *dev,
>  	return status;
>  }
>  
> -static int scale_sensor_value(u32 value, int data_type)
> +static long scale_sensor_value(s32 value, int data_type)
>  {
>  	switch (data_type) {
>  	case hwmon_curr:
> @@ -476,7 +476,7 @@ static int scale_sensor_value(u32 value, int data_type)
>  
>  static int get_cached_value_or_update(const struct device *dev,
>  				      int sensor_index,
> -				      struct ec_sensors_data *state, u32 *value)
> +				      struct ec_sensors_data *state, s32 *value)
>  {
>  	if (time_after(jiffies, state->last_updated + HZ)) {
>  		if (update_ec_sensors(dev, state)) {
> @@ -499,7 +499,7 @@ static int asus_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  			      u32 attr, int channel, long *val)
>  {
>  	int ret;
> -	u32 value = 0;
> +	s32 value = 0;
>  
>  	struct ec_sensors_data *state = dev_get_drvdata(dev);
>  	int sidx = find_ec_sensor_index(state, type, channel);