dell-smm-hwmon: security problems

DISCUSSION Pali Rohár 3 participants 7 messages Jun 8, 2016 Hardware monitoring

Share & tools

Pali Rohár

author

2016-06-08 09:57 UTC #0 lore raw

Hello!

Mario wrote me about two I think security problems in dell-smm-hwmon driver and I would like to ask you, how to fix them.

1) File /proc/i8k (exists only when kernel is compiled with CONFIG_I8K) exports DMI_PRODUCT_SERIAL and it can be read by ordinary user, without root permission. Normally DMI_PRODUCT_SERIAL can be read from sysfs file /sys/class/dmi/id/product_serial but only by root user.

2) Via /proc/i8k ordinary user can set fan speed. This is because how "restricted" parameter and variable works. Setting fan speed by normal non-root user can be dangerous, e.g. malicious application under user "nobody" could take control of fans.

Do you have idea how to fix these problems? Just to note that /proc/i8k has stable kernel ABI and changing it will break all existing i8k* applications. But /proc/i8k is there only for old legacy laptops (year 2000).

There is module parameter "restricted" with default value false and
description: "Allow fan control if SYS_ADMIN capability set". Current code do:

	case I8K_SET_FAN:
		if (restricted && !capable(CAP_SYS_ADMIN))
			return -EPERM;

For me description is a bit ambiguous. What about setting "restricted" by default to true and updating description to something like this?

"Disallow fan control when SYS_ADMIN capability is not set (default: 1)"

Signature · 2 lines

Pali Rohár
[email protected]

Guenter Roeck

maintainer

2016-06-08 13:24 UTC #1 lore raw

Quoted 24 lines · Pali Rohár

Hello!
Mario wrote me about two I think security problems in dell-smm-hwmon driver and I would like to ask you, how to fix them.
1) File /proc/i8k (exists only when kernel is compiled with CONFIG_I8K) exports DMI_PRODUCT_SERIAL and it can be read by ordinary user, without root permission. Normally DMI_PRODUCT_SERIAL can be read from sysfs file /sys/class/dmi/id/product_serial but only by root user.
2) Via /proc/i8k ordinary user can set fan speed. This is because how "restricted" parameter and variable works. Setting fan speed by normal non-root user can be dangerous, e.g. malicious application under user "nobody" could take control of fans.
Do you have idea how to fix these problems? Just to note that /proc/i8k has stable kernel ABI and changing it will break all existing i8k* applications. But /proc/i8k is there only for old legacy laptops (year 2000).
There is module parameter "restricted" with default value false and
description: "Allow fan control if SYS_ADMIN capability set". Current code do:

Quoted 9 lines

	case I8K_SET_FAN:
		if (restricted && !capable(CAP_SYS_ADMIN))
			return -EPERM;
For me description is a bit ambiguous. What about setting "restricted" by default to true and updating description to something like this?
"Disallow fan control when SYS_ADMIN capability is not set (default: 1)"

Sure. I am sure that someone will complain (we learned just recently that people still use the old commands, after all), but then the old behavior can be restored by setting the flag to 0.

I would not use a double negative to describe it. Why not just something like "Allow fan control only if SYS_ADMIN capability set (default 1)" ?

Guenter

Pali Rohár

author

2016-06-08 13:55 UTC #2 lore raw

Quoted 35 lines · On Wednesday 08 June 2016 15:24:10 Guenter Roeck wrote:

Quoted 34 lines · Pali Rohár
Hello!
Mario wrote me about two I think security problems in dell-smm-hwmon driver and I would like to ask you, how to fix them.
1) File /proc/i8k (exists only when kernel is compiled with CONFIG_I8K) exports DMI_PRODUCT_SERIAL and it can be read by ordinary user, without root permission. Normally DMI_PRODUCT_SERIAL can be read from sysfs file /sys/class/dmi/id/product_serial but only by root user.
2) Via /proc/i8k ordinary user can set fan speed. This is because how "restricted" parameter and variable works. Setting fan speed by normal non-root user can be dangerous, e.g. malicious application under user "nobody" could take control of fans.
Do you have idea how to fix these problems? Just to note that /proc/i8k has stable kernel ABI and changing it will break all existing i8k* applications. But /proc/i8k is there only for old legacy laptops (year 2000).
There is module parameter "restricted" with default value false and
description: "Allow fan control if SYS_ADMIN capability set". Current code do:
	case I8K_SET_FAN:
		if (restricted && !capable(CAP_SYS_ADMIN))
			return -EPERM;
For me description is a bit ambiguous. What about setting "restricted" by default to true and updating description to something like this?

Quoted 6 lines

Quoted 2 lines
"Disallow fan control when SYS_ADMIN capability is not set (default: 1)"
Sure. I am sure that someone will complain (we learned just recently that people still use the old commands, after all), but then the old behavior can be restored by setting the flag to 0.

Either setting that flag to 0 or running that tool under root or with capability CAP_SYS_ADMIN.

Quoted 3 lines

I would not use a double negative to describe it. Why not just something like "Allow fan control only if SYS_ADMIN capability set (default 1)" ?

I was thinking about that description too, but there is problem with meaning too...

0 means fan control is allowed for any user
1 means fan control is allowed only for CAP_SYS_ADMIN

Description should be unambiguous for situation when flag is set to 0.

===

And do you have idea what to do with problem 1)?

Signature · 2 lines

Pali Rohár
[email protected]

Guenter Roeck

maintainer

2016-06-08 17:37 UTC #3 lore raw

Quoted 50 lines · Pali Rohár

Quoted 35 lines · On Wednesday 08 June 2016 15:24:10 Guenter Roeck wrote:
Quoted 34 lines · Pali Rohár
Hello!
Mario wrote me about two I think security problems in dell-smm-hwmon driver and I would like to ask you, how to fix them.
1) File /proc/i8k (exists only when kernel is compiled with CONFIG_I8K) exports DMI_PRODUCT_SERIAL and it can be read by ordinary user, without root permission. Normally DMI_PRODUCT_SERIAL can be read from sysfs file /sys/class/dmi/id/product_serial but only by root user.
2) Via /proc/i8k ordinary user can set fan speed. This is because how "restricted" parameter and variable works. Setting fan speed by normal non-root user can be dangerous, e.g. malicious application under user "nobody" could take control of fans.
Do you have idea how to fix these problems? Just to note that /proc/i8k has stable kernel ABI and changing it will break all existing i8k* applications. But /proc/i8k is there only for old legacy laptops (year 2000).
There is module parameter "restricted" with default value false and
description: "Allow fan control if SYS_ADMIN capability set". Current code do:
	case I8K_SET_FAN:
		if (restricted && !capable(CAP_SYS_ADMIN))
			return -EPERM;
For me description is a bit ambiguous. What about setting "restricted" by default to true and updating description to something like this?
Quoted 6 lines
Quoted 2 lines
"Disallow fan control when SYS_ADMIN capability is not set (default: 1)"
Sure. I am sure that someone will complain (we learned just recently that people still use the old commands, after all), but then the old behavior can be restored by setting the flag to 0.
Either setting that flag to 0 or running that tool under root or with capability CAP_SYS_ADMIN.
Quoted 3 lines
I would not use a double negative to describe it. Why not just something like "Allow fan control only if SYS_ADMIN capability set (default 1)" ?

Quoted 8 lines

I was thinking about that description too, but there is problem with meaning too...
0 means fan control is allowed for any user
1 means fan control is allowed only for CAP_SYS_ADMIN
Description should be unambiguous for situation when flag is set to 0.

Sorry, I don't understand how a double negation "disallow ... if not set" would make things less ambiguous than "allow ... only if set".

Quoted 4 lines

===
And do you have idea what to do with problem 1)?

If you really want to do something about it, you could whiteout the serial number if CAP_SYS_ADMIN is not set.

Guenter

Austin S. Hemmelgarn

2016-06-08 17:54 UTC #4 lore raw

Quoted 54 lines · Guenter Roeck

Quoted 53 lines · Pali Rohár
Quoted 35 lines · On Wednesday 08 June 2016 15:24:10 Guenter Roeck wrote:
Quoted 34 lines · Pali Rohár
Hello!
Mario wrote me about two I think security problems in dell-smm-hwmon driver and I would like to ask you, how to fix them.
1) File /proc/i8k (exists only when kernel is compiled with CONFIG_I8K) exports DMI_PRODUCT_SERIAL and it can be read by ordinary user, without root permission. Normally DMI_PRODUCT_SERIAL can be read from sysfs file /sys/class/dmi/id/product_serial but only by root user.
2) Via /proc/i8k ordinary user can set fan speed. This is because how "restricted" parameter and variable works. Setting fan speed by normal non-root user can be dangerous, e.g. malicious application under user "nobody" could take control of fans.
Do you have idea how to fix these problems? Just to note that /proc/i8k has stable kernel ABI and changing it will break all existing i8k* applications. But /proc/i8k is there only for old legacy laptops (year 2000).
There is module parameter "restricted" with default value false and
description: "Allow fan control if SYS_ADMIN capability set". Current code do:
	case I8K_SET_FAN:
		if (restricted && !capable(CAP_SYS_ADMIN))
			return -EPERM;
For me description is a bit ambiguous. What about setting "restricted" by default to true and updating description to something like this?
Quoted 6 lines
Quoted 2 lines
"Disallow fan control when SYS_ADMIN capability is not set (default: 1)"
Sure. I am sure that someone will complain (we learned just recently that people still use the old commands, after all), but then the old behavior can be restored by setting the flag to 0.
Either setting that flag to 0 or running that tool under root or with capability CAP_SYS_ADMIN.
Quoted 3 lines
I would not use a double negative to describe it. Why not just something like "Allow fan control only if SYS_ADMIN capability set (default 1)" ?
I was thinking about that description too, but there is problem with meaning too...

Quoted 7 lines

Quoted 5 lines
0 means fan control is allowed for any user
1 means fan control is allowed only for CAP_SYS_ADMIN
Description should be unambiguous for situation when flag is set to 0.
Sorry, I don't understand how a double negation "disallow ... if not set" would make things less ambiguous than "allow ... only if set".

Double negatives become ambiguous when you start to deal with the possibility of translation or working with people who are not native speakers of the language in question. In English they're traditionally considered bad grammar, while in most other languages they are used for emphasis and nothing else, and thus are considered by some people to be bad form in technical documentation.

Given this particular case, it would probably be the least ambiguous to say:
Restrict fan control to CAP_SYS_ADMIN

Pali Rohár

author

2016-06-08 18:10 UTC #5 lore raw

Quoted 75 lines · On Wednesday 08 June 2016 19:54:35 Austin S. Hemmelgarn wrote:

Quoted 60 lines · Guenter Roeck
Quoted 59 lines · Pali Rohár
Quoted 38 lines · On Wednesday 08 June 2016 15:24:10 Guenter Roeck wrote:
Quoted 37 lines · Pali Rohár
Hello!
Mario wrote me about two I think security problems in dell-smm-hwmon driver and I would like to ask you, how to fix them.
1) File /proc/i8k (exists only when kernel is compiled with CONFIG_I8K) exports DMI_PRODUCT_SERIAL and it can be read by ordinary user, without root permission. Normally DMI_PRODUCT_SERIAL can be read from sysfs file /sys/class/dmi/id/product_serial but only by root user.
2) Via /proc/i8k ordinary user can set fan speed. This is because how "restricted" parameter and variable works. Setting fan speed by normal non-root user can be dangerous, e.g. malicious application under user "nobody" could take control of fans.
Do you have idea how to fix these problems? Just to note that /proc/i8k has stable kernel ABI and changing it will break all existing i8k* applications. But /proc/i8k is there only for old legacy laptops (year 2000).
There is module parameter "restricted" with default value false and description: "Allow fan control if SYS_ADMIN capability set".
Current code do:
	case I8K_SET_FAN:
		if (restricted && !capable(CAP_SYS_ADMIN))
		
			return -EPERM;
For me description is a bit ambiguous. What about setting "restricted" by default to true and updating description to something like this?
Quoted 6 lines
Quoted 2 lines
"Disallow fan control when SYS_ADMIN capability is not set (default: 1)"
Sure. I am sure that someone will complain (we learned just recently that people still use the old commands, after all), but then the old behavior can be restored by setting the flag to 0.
Either setting that flag to 0 or running that tool under root or with capability CAP_SYS_ADMIN.
Quoted 3 lines
I would not use a double negative to describe it. Why not just something like "Allow fan control only if SYS_ADMIN capability set (default 1)" ?
I was thinking about that description too, but there is problem with meaning too...
0 means fan control is allowed for any user
1 means fan control is allowed only for CAP_SYS_ADMIN
Quoted 6 lines
Quoted 2 lines
Description should be unambiguous for situation when flag is set to 0.
Sorry, I don't understand how a double negation "disallow ... if not set" would make things less ambiguous than "allow ... only if set".
Double negatives become ambiguous when you start to deal with the possibility of translation or working with people who are not native speakers of the language in question. In English they're traditionally considered bad grammar, while in most other languages they are used for emphasis and nothing else, and thus are considered by some people to be bad form in technical documentation.

Quoted 2 lines

Given this particular case, it would probably be the least ambiguous to say: Restrict fan control to CAP_SYS_ADMIN

Thank you, this is really better!

Signature · 2 lines

Pali Rohár
[email protected]

Pali Rohár

author

2016-06-08 18:10 UTC #6 lore raw

In reply to Guenter Roeck

Quoted 5 lines · On Wednesday 08 June 2016 19:37:43 Guenter Roeck wrote:

Quoted 1 line · Pali Rohár
And do you have idea what to do with problem 1)?
If you really want to do something about it, you could whiteout the serial number if CAP_SYS_ADMIN is not set.

Ok, that sounds reasonable.

Signature · 2 lines

Pali Rohár
[email protected]