linux-kernel - Re: [PATCH 2/6] hwmon: (dell-smm) Restrict fan control and serial number to CAP_SYS

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 18 Jun 2016 13:00:41 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Pali Rohár <pali.rohar@...il.com>,
	Jean Delvare <jdelvare@...e.com>,
	Jan C Peters <jcpeters89@...il.com>,
	Thorsten Leemhuis <fedora@...mhuis.info>,
	David Santamaría Rogado <howl.nsp@...il.com>,
	Peter Saunderson <peteasa@...il.com>,
	Tolga Cakir <cevelnet@...il.com>,
	"Austin S. Hemmelgarn" <ahferroin7@...il.com>,
	Mario_Limonciello@...l.com,
	Gabriele Mazzotta <gabriele.mzt@...il.com>,
	Michał Kępień 
	<kernel@...pniu.pl>, Dakota Whipple <dakotajaywhipple@...il.com>,
	Leon Yu <leon@...nyu.net>
Cc:	linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6] hwmon: (dell-smm) Restrict fan control and serial
 number to CAP_SYS_ADMIN by default

On 06/17/2016 03:54 PM, Pali Rohár wrote:
> For security reasons ordinary user must not be able to control fan speed
> via /proc/i8k by default. Some malicious software running under "nobody"
> user could be able to turn fan off and cause HW problems. So this patch
> changes default value of "restricted" parameter to 1.
>
> Also restrict reading of DMI_PRODUCT_SERIAL from /proc/i8k via "restricted"
> parameter. It is because non root user cannot read DMI_PRODUCT_SERIAL from
> sysfs file /sys/class/dmi/id/product_serial.
>
> Old non secure behaviour of file /proc/i8k can be achieved by loading this
> module with "restricted" parameter set to 0.
>
> Note that this patch has effects only for kernels compiled with CONFIG_I8K
> and only for file /proc/i8k. Hwmon interface provided by this driver was
> not changed and root access for setting fan speed was needed also before.
>
> Reported-by: Mario Limonciello <Mario_Limonciello@...l.com>
> Signed-off-by: Pali Rohár <pali.rohar@...il.com>
> Cc: stable@...r.kernel.org # will need backport

Applied.

Guenter

> ---
>   drivers/hwmon/dell-smm-hwmon.c |   19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 480b2fa..c8bd3fdd 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -67,6 +67,7 @@
>
>   static DEFINE_MUTEX(i8k_mutex);
>   static char bios_version[4];
> +static char bios_machineid[16];
>   static struct device *i8k_hwmon_dev;
>   static u32 i8k_hwmon_flags;
>   static uint i8k_fan_mult = I8K_FAN_MULT;
> @@ -95,13 +96,13 @@ module_param(ignore_dmi, bool, 0);
>   MODULE_PARM_DESC(ignore_dmi, "Continue probing hardware even if DMI data does not match");
>
>   #if IS_ENABLED(CONFIG_I8K)
> -static bool restricted;
> +static bool restricted = true;
>   module_param(restricted, bool, 0);
> -MODULE_PARM_DESC(restricted, "Allow fan control if SYS_ADMIN capability set");
> +MODULE_PARM_DESC(restricted, "Restrict fan control and serial number to CAP_SYS_ADMIN (default: 1)");
>
>   static bool power_status;
>   module_param(power_status, bool, 0600);
> -MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k");
> +MODULE_PARM_DESC(power_status, "Report power status in /proc/i8k (default: 0)");
>   #endif
>
>   static uint fan_mult;
> @@ -397,9 +398,11 @@ i8k_ioctl_unlocked(struct file *fp, unsigned int cmd, unsigned long arg)
>   		break;
>
>   	case I8K_MACHINE_ID:
> -		memset(buff, 0, 16);
> -		strlcpy(buff, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
> -			sizeof(buff));
> +		if (restricted && !capable(CAP_SYS_ADMIN))
> +			return -EPERM;
> +
> +		memset(buff, 0, sizeof(buff));
> +		strlcpy(buff, bios_machineid, sizeof(buff));
>   		break;
>
>   	case I8K_FN_STATUS:
> @@ -516,7 +519,7 @@ static int i8k_proc_show(struct seq_file *seq, void *offset)
>   	seq_printf(seq, "%s %s %s %d %d %d %d %d %d %d\n",
>   		   I8K_PROC_FMT,
>   		   bios_version,
> -		   i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
> +		   (restricted && !capable(CAP_SYS_ADMIN)) ? "-1" : bios_machineid,
>   		   cpu_temp,
>   		   left_fan, right_fan, left_speed, right_speed,
>   		   ac_power, fn_key);
> @@ -985,6 +988,8 @@ static int __init i8k_probe(void)
>
>   	strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),
>   		sizeof(bios_version));
> +	strlcpy(bios_machineid, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
> +		sizeof(bios_machineid));
>
>   	/*
>   	 * Get SMM Dell signature
>