diff options
author | Al Viro <viro@zeniv.linux.org.uk> | 2017-09-29 14:07:09 -0400 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2017-11-10 08:48:20 -0500 |
commit | 50271d388fa4171565a8fdb995b3255f067e150d (patch) | |
tree | 624ff5989b35fc9d7dfd6b64c9084a0ffd06000f /drivers | |
parent | a0dbef33863349de5313dff62982d309ded98190 (diff) | |
download | linux-stable-50271d388fa4171565a8fdb995b3255f067e150d.tar.gz linux-stable-50271d388fa4171565a8fdb995b3255f067e150d.tar.bz2 linux-stable-50271d388fa4171565a8fdb995b3255f067e150d.zip |
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 <viro@zeniv.linux.org.uk>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/staging/pi433/pi433_if.c | 92 |
1 files changed, 14 insertions, 78 deletions
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); |