linux-kernel - Re: [lm-sensors] hwmon/f75375s.c: buggy if()

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:	Fri, 26 Oct 2007 10:36:47 +0200
From:	Jean Delvare <khali@...ux-fr.org>
To:	Riku Voipio <riku.voipio@....fi>
Cc:	"Mark M. Hoffman" <mhoffman@...htlink.com>,
	Adrian Bunk <bunk@...nel.org>, linux-kernel@...r.kernel.org,
	lm-sensors@...sensors.org
Subject: Re: [lm-sensors] hwmon/f75375s.c: buggy if()

Hi Riku,

On Thu, 25 Oct 2007 14:48:14 +0300, Riku Voipio wrote:
> On Wed, Oct 24, 2007 at 10:25:29PM -0400, Mark M. Hoffman wrote:
> > * Riku Voipio <riku.voipio@....fi> [2007-10-24 14:50:34 +0300]:
> > > On Fri, Oct 19, 2007 at 02:37:54PM +0200, Jean Delvare wrote:
> > > > Riku, can you please submit a patch fixing this? The attribute should
> > > > be declared read-only, and then you can use sysfs_chmod_file() to
> > > > change it to read-write where supported. 
> 
> > > Thanks, this was good suggestion. Patch attached.
> 
> > No patch?
> 
> Let's try again..

> Fix value check in set_pwm_mode(). Instead of checking for
> chip variant there, make pwmX_mode sysfs nodes only writable
> on f75375 variant.
> 
> Signed-off-by: Riku Voipio <riku.voipio@...ial.fi>
> ---
>  drivers/hwmon/f75375s.c |   19 ++++++++++++++++---
>  1 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c
> index 13a0413..d3b7932 100644
> --- a/drivers/hwmon/f75375s.c
> +++ b/drivers/hwmon/f75375s.c
> @@ -323,7 +323,7 @@ static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *attr,
>  	int val = simple_strtoul(buf, NULL, 10);
>  	u8 conf = 0;
>  
> -	if (val != 0 || val != 1 || data->kind == f75373)
> +	if (!(val == 0 || val == 1))
>  		return -EINVAL;
>  
>  	mutex_lock(&data->update_lock);
> @@ -529,13 +529,13 @@ static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO|S_IWUSR,
>  	show_pwm, set_pwm, 0);
>  static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO|S_IWUSR,
>  	show_pwm_enable, set_pwm_enable, 0);
> -static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO|S_IWUSR,
> +static SENSOR_DEVICE_ATTR(pwm1_mode, S_IRUGO,
>  	show_pwm_mode, set_pwm_mode, 0);
>  static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR,
>  	show_pwm, set_pwm, 1);
>  static SENSOR_DEVICE_ATTR(pwm2_enable, S_IRUGO|S_IWUSR,
>  	show_pwm_enable, set_pwm_enable, 1);
> -static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO|S_IWUSR,
> +static SENSOR_DEVICE_ATTR(pwm2_mode, S_IRUGO,
>  	show_pwm_mode, set_pwm_mode, 1);
>  
>  static struct attribute *f75375_attributes[] = {
> @@ -655,6 +655,19 @@ static int f75375_detect(struct i2c_adapter *adapter, int address, int kind)
>  	if ((err = sysfs_create_group(&client->dev.kobj, &f75375_group)))
>  		goto exit_detach;
>  
> +	if (kind == f75375) {
> +		err = sysfs_chmod_file(&client->dev.kobj,
> +			&sensor_dev_attr_pwm1_mode.dev_attr.attr,
> +			S_IRUGO | S_IWUSR);
> +		if (err)
> +			goto exit_detach;
> +		err = sysfs_chmod_file(&client->dev.kobj,
> +			&sensor_dev_attr_pwm2_mode.dev_attr.attr,
> +			S_IRUGO | S_IWUSR);
> +		if (err)
> +			goto exit_detach;
> +	}
> +
>  	data->hwmon_dev = hwmon_device_register(&client->dev);
>  	if (IS_ERR(data->hwmon_dev)) {
>  		err = PTR_ERR(data->hwmon_dev);

Patch looks correct, however it doesn't apply on top of Mark's tree. I
was able to get it to apply by reverting "(f75375s) fix pwm mode
setting" first, but then the build fails. Presumably the other f75375s
patches interact badly. Can you please respin this patch on top of
Mark's tree (i.e. on top the the 4 other f75375s patches you sent since
the -rc1 merge)? Thanks.

-- 
Jean Delvare
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/