From 50271d388fa4171565a8fdb995b3255f067e150d Mon Sep 17 00:00:00 2001
From: Al Viro <viro@zeniv.linux.org.uk>
Date: Fri, 29 Sep 2017 14:07:09 -0400
Subject: [PATCH] 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>
---
 drivers/staging/pi433/pi433_if.c | 92 +++++---------------------------
 1 file 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);