On Wed, Jun 08, 2016 at 03:55:48PM +0200, Pali Rohár wrote: > On Wednesday 08 June 2016 15:24:10 Guenter Roeck wrote: > > On 06/08/2016 02:57 AM, Pali Rohár wrote: > > > 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)" > > > > 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. > > > 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. > Sorry, I don't understand how a double negation "disallow ... if not set" would make things less ambiguous than "allow ... only if set". > === > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html