netdev - Re: [PATCH net-next 3/3] mlxsw: core: Add support for negative temperature readout

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: <20190623154407.GE28942@lunn.ch>
Date:   Sun, 23 Jun 2019 17:44:07 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, jiri@...lanox.com,
        mlxsw@...lanox.com, Vadim Pasternak <vadimp@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>
Subject: Re: [PATCH net-next 3/3] mlxsw: core: Add support for negative
 temperature readout

> --- a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
> @@ -52,8 +52,7 @@ static ssize_t mlxsw_hwmon_temp_show(struct device *dev,
>  			container_of(attr, struct mlxsw_hwmon_attr, dev_attr);
>  	struct mlxsw_hwmon *mlxsw_hwmon = mlwsw_hwmon_attr->hwmon;
>  	char mtmp_pl[MLXSW_REG_MTMP_LEN];
> -	unsigned int temp;
> -	int index;
> +	int temp, index;
>  	int err;
>  
>  	index = mlxsw_hwmon_get_attr_index(mlwsw_hwmon_attr->type_index,
> @@ -65,7 +64,7 @@ static ssize_t mlxsw_hwmon_temp_show(struct device *dev,
>  		return err;
>  	}
>  	mlxsw_reg_mtmp_unpack(mtmp_pl, &temp, NULL, NULL);
> -	return sprintf(buf, "%u\n", temp);
> +	return sprintf(buf, "%d\n", temp);
>  }

If you had used the hwmon core, rather than implementing it yourself,
you could of avoided this part of the bug.

>  static ssize_t mlxsw_hwmon_temp_rst_store(struct device *dev,
> @@ -215,8 +213,8 @@ static ssize_t mlxsw_hwmon_module_temp_show(struct device *dev,
>  			container_of(attr, struct mlxsw_hwmon_attr, dev_attr);
>  	struct mlxsw_hwmon *mlxsw_hwmon = mlwsw_hwmon_attr->hwmon;
>  	char mtmp_pl[MLXSW_REG_MTMP_LEN];
> -	unsigned int temp;
>  	u8 module;
> +	int temp;
>  	int err;
>  
>  	module = mlwsw_hwmon_attr->type_index - mlxsw_hwmon->sensor_count;

I think you missed changing the %u to %d in this function.

> @@ -519,14 +519,14 @@ static int mlxsw_thermal_module_temp_get(struct thermal_zone_device *tzdev,
>  		return 0;
>  	}
>  	mlxsw_reg_mtmp_unpack(mtmp_pl, &temp, NULL, NULL);
> -	*p_temp = (int) temp;
> +	*p_temp = temp;
>  
>  	if (!temp)
>  		return 0;
>  
>  	/* Update trip points. */
>  	err = mlxsw_thermal_module_trips_update(dev, thermal->core, tz);
> -	if (!err)
> +	if (!err && temp > 0)
>  		mlxsw_thermal_tz_score_update(thermal, tzdev, tz->trips, temp);

Why the > 0?

    Andrew