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 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) --- 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 . */ +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