Implement ->proc_ioctl_buf() so procfs copies the arguments in before it pins the entry and back out after unpinning it. The command numbers in <uapi/linux/i8k.h> encode the wrong sizes, so the table lists what is really copied: an int or the 16 bytes of the machine id out, and one or two ints in.
A NULL argument fails with -EFAULT now instead of -EINVAL, and so does a bad pointer to SET_FAN from an unprivileged task, which got -EPERM before, since procfs copies the argument in before the entry's own checks run. 32-bit tasks can use the commands whose numbers don't depend on the size of size_t. Signed-off-by: Christian Brauner (Amutable) <[email protected]> --- drivers/hwmon/dell-smm-hwmon.c | 48 +++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c index 492b2b6c6d7e..67b8c6e5752f 100644 --- a/drivers/hwmon/dell-smm-hwmon.c +++ b/drivers/hwmon/dell-smm-hwmon.c @@ -549,16 +549,13 @@ static int i8k_get_power_status(const struct dell_smm_data *data) * Procfs interface */ -static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) +static long i8k_ioctl(struct file *fp, unsigned int cmd, void *arg) { struct dell_smm_data *data = pde_data(file_inode(fp)); - int __user *argp = (int __user *)arg; + int *argp = arg; int speed, err; int val = 0; - if (!argp) - return -EINVAL; - switch (cmd) { case I8K_BIOS_VERSION: if (!isdigit(data->bios_version[0]) || !isdigit(data->bios_version[1]) || @@ -568,17 +565,13 @@ static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) val = (data->bios_version[0] << 16) | (data->bios_version[1] << 8) | data->bios_version[2]; - if (copy_to_user(argp, &val, sizeof(val))) - return -EFAULT; - + *argp = val; return 0; case I8K_MACHINE_ID: if (restricted && !capable(CAP_SYS_ADMIN)) return -EPERM; - if (copy_to_user(argp, data->bios_machineid, sizeof(data->bios_machineid))) - return -EFAULT; - + memcpy(argp, data->bios_machineid, sizeof(data->bios_machineid)); return 0; case I8K_FN_STATUS: val = i8k_get_fn_status(data); @@ -593,8 +586,7 @@ static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) break; case I8K_GET_SPEED: - if (copy_from_user(&val, argp, sizeof(int))) - return -EFAULT; + val = *argp; if (val > U8_MAX || val < 0) return -EINVAL; @@ -603,8 +595,7 @@ static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) break; case I8K_GET_FAN: - if (copy_from_user(&val, argp, sizeof(int))) - return -EFAULT; + val = *argp; if (val > U8_MAX || val < 0) return -EINVAL; @@ -616,14 +607,11 @@ static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) if (restricted && !capable(CAP_SYS_ADMIN)) return -EPERM; - if (copy_from_user(&val, argp, sizeof(int))) - return -EFAULT; - + val = argp[0]; if (val > U8_MAX || val < 0) return -EINVAL; - if (copy_from_user(&speed, argp + 1, sizeof(int))) - return -EFAULT; + speed = argp[1]; speed = clamp_val(speed, 0, data->i8k_fan_max); @@ -643,12 +631,22 @@ static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) if (val < 0) return val; - if (copy_to_user(argp, &val, sizeof(int))) - return -EFAULT; - + *argp = val; return 0; } +/* The command numbers don't encode what is copied, see <uapi/linux/i8k.h>. */ +static const struct proc_ioctl_arg i8k_ioctl_args[] = { + PROC_IOCTL_ARG(I8K_BIOS_VERSION, 0, sizeof(int)), + PROC_IOCTL_ARG(I8K_MACHINE_ID, 0, sizeof_field(struct dell_smm_data, bios_machineid)), + PROC_IOCTL_ARG(I8K_FN_STATUS, 0, sizeof(int)), + PROC_IOCTL_ARG(I8K_POWER_STATUS, 0, sizeof(int)), + PROC_IOCTL_ARG(I8K_GET_TEMP, 0, sizeof(int)), + PROC_IOCTL_ARG(I8K_GET_SPEED, sizeof(int), sizeof(int)), + PROC_IOCTL_ARG(I8K_GET_FAN, sizeof(int), sizeof(int)), + PROC_IOCTL_ARG(I8K_SET_FAN, 2 * sizeof(int), sizeof(int)), +}; + /* * Print the information for /proc/i8k. */ @@ -696,7 +694,9 @@ static int i8k_proc_show(struct seq_file *seq, void *offset) static const struct proc_ops i8k_proc_ops = { .proc_show = i8k_proc_show, - .proc_ioctl = i8k_ioctl, + .proc_ioctl_buf = i8k_ioctl, + .proc_ioctl_args = i8k_ioctl_args, + .proc_nr_ioctl_args = ARRAY_SIZE(i8k_ioctl_args), }; static void i8k_exit_procfs(void *param) -- 2.53.0
