From 50271d388fa4171565a8fdb995b3255f067e150d Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 29 Sep 2017 14:07:09 -0400 Subject: pi433: sanitize ioctl a) those access_ok() are pointless b) guarding against the ioctl number declaration changes in that way is pointless, especially since we _know_ the size of object we want to copy. [folded braino fixes from Colin Ian King and Dan Carpenter] Signed-off-by: Al Viro --- drivers/staging/pi433/pi433_if.c | 92 ++++++---------------------------------- 1 file changed, 14 insertions(+), 78 deletions(-) (limited to 'drivers/staging') diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 93c01680f016..67c6b540fc4a 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -767,32 +767,15 @@ abort: static long pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { - int err = 0; int retval = 0; struct pi433_instance *instance; struct pi433_device *device; - u32 tmp; + void __user *argp = (void __user *)arg; /* Check type and command number */ if (_IOC_TYPE(cmd) != PI433_IOC_MAGIC) return -ENOTTY; - /* Check access direction once here; don't repeat below. - * IOC_DIR is from the user perspective, while access_ok is - * from the kernel perspective; so they look reversed. - */ - if (_IOC_DIR(cmd) & _IOC_READ) - err = !access_ok(VERIFY_WRITE, - (void __user *)arg, - _IOC_SIZE(cmd)); - - if (err == 0 && _IOC_DIR(cmd) & _IOC_WRITE) - err = !access_ok(VERIFY_READ, - (void __user *)arg, - _IOC_SIZE(cmd)); - if (err) - return -EFAULT; - /* TODO? guard against device removal before, or while, * we issue this ioctl. --> device_get() */ @@ -804,80 +787,33 @@ pi433_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) switch (cmd) { case PI433_IOC_RD_TX_CFG: - tmp = _IOC_SIZE(cmd); - if ( (tmp == 0) || ((tmp % sizeof(struct pi433_tx_cfg)) != 0) ) - { - retval = -EINVAL; - break; - } - - if (__copy_to_user((void __user *)arg, - &instance->tx_cfg, - tmp)) - { - retval = -EFAULT; - break; - } - + if (copy_to_user(argp, &instance->tx_cfg, + sizeof(struct pi433_tx_cfg))) + return -EFAULT; break; case PI433_IOC_WR_TX_CFG: - tmp = _IOC_SIZE(cmd); - if ( (tmp == 0) || ((tmp % sizeof(struct pi433_tx_cfg)) != 0) ) - { - retval = -EINVAL; - break; - } - - if (__copy_from_user(&instance->tx_cfg, - (void __user *)arg, - tmp)) - { - retval = -EFAULT; - break; - } - + if (copy_from_user(&instance->tx_cfg, argp, + sizeof(struct pi433_tx_cfg))) + return -EFAULT; break; - case PI433_IOC_RD_RX_CFG: - tmp = _IOC_SIZE(cmd); - if ( (tmp == 0) || ((tmp % sizeof(struct pi433_rx_cfg)) != 0) ) { - retval = -EINVAL; - break; - } - - if (__copy_to_user((void __user *)arg, - &device->rx_cfg, - tmp)) - { - retval = -EFAULT; - break; - } - + if (copy_to_user(argp, &device->rx_cfg, + sizeof(struct pi433_rx_cfg))) + return -EFAULT; break; case PI433_IOC_WR_RX_CFG: - tmp = _IOC_SIZE(cmd); mutex_lock(&device->rx_lock); /* during pendig read request, change of config not allowed */ if (device->rx_active) { - retval = -EAGAIN; mutex_unlock(&device->rx_lock); - break; + return -EAGAIN; } - if ( (tmp == 0) || ((tmp % sizeof(struct pi433_rx_cfg)) != 0) ) { - retval = -EINVAL; + if (copy_from_user(&device->rx_cfg, argp, + sizeof(struct pi433_rx_cfg))) { mutex_unlock(&device->rx_lock); - break; - } - - if (__copy_from_user(&device->rx_cfg, - (void __user *)arg, - tmp)) - { - retval = -EFAULT; - mutex_unlock(&device->rx_lock); - break; + return -EFAULT; } mutex_unlock(&device->rx_lock); -- cgit v1.2.3-59-g8ed1b