Skip to contentSkip to conversation

I2C: Fix incorrect sysfs file permissions in it87 and

PATCH New Jean Delvare 2 participants 3 messages May 19, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 I2C: Fix incorrect sysfs file permissions in it87 and

Hi Greg,

The it87 and via686a hardware monitoring drivers each create a sysfs file named "alarms" in R/W mode, while they should really create it in read-only mode. Since we don't provide a store function for these files, write attempts to these files will do something undefined (I guess) and bad (I am sure). My own try resulted in a locked terminal (where I attempted the write) and a 100% CPU load until next reboot.

As a side note, wouldn't it make sense to check, when creating sysfs files, that readable files have a non-NULL show method, and writable files have a non-NULL store method? I know drivers are not supposed to do stupid things, but there is already a BUG_ON for several conditions in sysfs_create_file, so maybe we could add two more?

Please apply,
thanks.

(I leave it to you to decide whether this is a candidate for -stable.)

drivers/i2c/chips/it87.c renamed from drivers/i2c/chips/it87.c.orig +1 -1
@@ -668,7 +668,7 @@
 	struct it87_data *data = it87_update_device(dev);
 	return sprintf(buf,"%d\n", ALARMS_FROM_REG(data->alarms));
 }
-static DEVICE_ATTR(alarms, S_IRUGO | S_IWUSR, show_alarms, NULL);
+static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 
 static ssize_t
 show_vrm_reg(struct device *dev, char *buf)
drivers/i2c/chips/via686a.c renamed from drivers/i2c/chips/via686a.c.orig +1 -1
@@ -574,7 +574,7 @@
 	struct via686a_data *data = via686a_update_device(dev);
 	return sprintf(buf,"%d\n", ALARMS_FROM_REG(data->alarms));
 }
-static DEVICE_ATTR(alarms, S_IRUGO | S_IWUSR, show_alarms, NULL);
+static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 
 /* The driver. I choose to use type i2c_driver, as at is identical to both
    smbus_driver and isa_driver, and clients could be of either kind */
Signature · 1 line
Jean Delvare
Applied
#1 lore raw

PATCH 1/1 I2C: Fix incorrect sysfs file permissions in it87 and

Quoted 9 lines · Jean Delvare

Hi Greg,

The it87 and via686a hardware monitoring drivers each create a sysfs file named "alarms" in R/W mode, while they should really create it in read-only mode. Since we don't provide a store function for these files, write attempts to these files will do something undefined (I guess) and bad (I am sure). My own try resulted in a locked terminal (where I attempted the write) and a 100% CPU load until next reboot.

Quoted 7 lines

As a side note, wouldn't it make sense to check, when creating sysfs files, that readable files have a non-NULL show method, and writable files have a non-NULL store method? I know drivers are not supposed to do stupid things, but there is already a BUG_ON for several conditions in sysfs_create_file, so maybe we could add two more?

Please apply,

Applied, thanks.

greg k-h

#2 lore raw

PATCH 1/1 I2C: Fix incorrect sysfs file permissions in it87 and

In reply to Jean Delvare
Quoted 15 lines · Jean Delvare

Hi Greg,

The it87 and via686a hardware monitoring drivers each create a sysfs file named "alarms" in R/W mode, while they should really create it in read-only mode. Since we don't provide a store function for these files, write attempts to these files will do something undefined (I guess) and bad (I am sure). My own try resulted in a locked terminal (where I attempted the write) and a 100% CPU load until next reboot.

As a side note, wouldn't it make sense to check, when creating sysfs files, that readable files have a non-NULL show method, and writable files have a non-NULL store method? I know drivers are not supposed to do stupid things, but there is already a BUG_ON for several conditions in sysfs_create_file, so maybe we could add two more?

Quoted 4 lines

Please apply,
thanks.

(I leave it to you to decide whether this is a candidate for -stable.)

I will queue it up for the next round of patches for -stable. In the future, please just send stuff like this to the [email protected] email address, it will get picked up sooner and easier that way.

thanks,

greg k-h