staging/rdma/hfi1: Return early from hfi1_ioctl parameter errors

Rather than have a switch in a large else clause make the parameter checks
return immediately.

Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

1 files changed, 173 insertions, 175 deletions

diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c
index 75b4bf4..205fc83 100644
--- a/drivers/staging/rdma/hfi1/diag.c
+++ b/drivers/staging/rdma/hfi1/diag.c
@@ -987,17 +987,15 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
 	if (!dd)
 		return -ENODEV;
 
-	spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
-
 	mode_flag = dd->hfi1_snoop.mode_flag;
 
 	if (((_IOC_DIR(cmd) & _IOC_READ)
 	    && !access_ok(VERIFY_WRITE, (void __user *)arg, _IOC_SIZE(cmd)))
 	    || ((_IOC_DIR(cmd) & _IOC_WRITE)
 	    && !access_ok(VERIFY_READ, (void __user *)arg, _IOC_SIZE(cmd)))) {
-		ret = -EFAULT;
+		return -EFAULT;
 	} else if (!capable(CAP_SYS_ADMIN)) {
-		ret = -EPERM;
+		return -EPERM;
 	} else if ((mode_flag & HFI1_PORT_CAPTURE_MODE) &&
 		   (cmd != HFI1_SNOOP_IOCCLEARQUEUE) &&
 		   (cmd != HFI1_SNOOP_IOCCLEARFILTER) &&
@@ -1008,208 +1006,208 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
 		 * 3.Set capture filter
 		 * Other are invalid.
 		 */
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&dd->hfi1_snoop.snoop_lock, flags);
+
+	switch (cmd) {
+	case HFI1_SNOOP_IOCSETLINKSTATE:
+		snoop_dbg("HFI1_SNOOP_IOCSETLINKSTATE is not valid");
 		ret = -EINVAL;
-	} else {
-		switch (cmd) {
-		case HFI1_SNOOP_IOCSETLINKSTATE:
-			snoop_dbg("HFI1_SNOOP_IOCSETLINKSTATE is not valid");
+		break;
+
+	case HFI1_SNOOP_IOCSETLINKSTATE_EXTRA:
+		memset(&link_info, 0, sizeof(link_info));
+
+		if (copy_from_user(&link_info,
+				   (struct hfi1_link_info __user *)arg,
+				   sizeof(link_info)))
+			ret = -EFAULT;
+
+		value = link_info.port_state;
+		index = link_info.port_number;
+		if (index > dd->num_pports - 1) {
 			ret = -EINVAL;
 			break;
+		}
 
-		case HFI1_SNOOP_IOCSETLINKSTATE_EXTRA:
-			memset(&link_info, 0, sizeof(link_info));
+		ppd = &dd->pport[index];
+		if (!ppd) {
+			ret = -EINVAL;
+			break;
+		}
 
-			if (copy_from_user(&link_info,
-				(struct hfi1_link_info __user *)arg,
-				sizeof(link_info)))
-				ret = -EFAULT;
+		/* What we want to transition to */
+		phys_state = (value >> 4) & 0xF;
+		link_state = value & 0xF;
+		snoop_dbg("Setting link state 0x%x", value);
 
-			value = link_info.port_state;
-			index = link_info.port_number;
-			if (index > dd->num_pports - 1) {
-				ret = -EINVAL;
+		switch (link_state) {
+		case IB_PORT_NOP:
+			if (phys_state == 0)
 				break;
-			}
-
-			ppd = &dd->pport[index];
-			if (!ppd) {
-				ret = -EINVAL;
-				break;
-			}
-
-			/* What we want to transition to */
-			phys_state = (value >> 4) & 0xF;
-			link_state = value & 0xF;
-			snoop_dbg("Setting link state 0x%x", value);
-
-			switch (link_state) {
-			case IB_PORT_NOP:
-				if (phys_state == 0)
-					break;
-					/* fall through */
-			case IB_PORT_DOWN:
-				switch (phys_state) {
-				case 0:
-					dev_state = HLS_DN_DOWNDEF;
-					break;
-				case 2:
-					dev_state = HLS_DN_POLL;
-					break;
-				case 3:
-					dev_state = HLS_DN_DISABLE;
-					break;
-				default:
-					ret = -EINVAL;
-					goto done;
-				}
-				ret = set_link_state(ppd, dev_state);
+				/* fall through */
+		case IB_PORT_DOWN:
+			switch (phys_state) {
+			case 0:
+				dev_state = HLS_DN_DOWNDEF;
 				break;
-			case IB_PORT_ARMED:
-				ret = set_link_state(ppd, HLS_UP_ARMED);
-				if (!ret)
-					send_idle_sma(dd, SMA_IDLE_ARM);
+			case 2:
+				dev_state = HLS_DN_POLL;
 				break;
-			case IB_PORT_ACTIVE:
-				ret = set_link_state(ppd, HLS_UP_ACTIVE);
-				if (!ret)
-					send_idle_sma(dd, SMA_IDLE_ACTIVE);
+			case 3:
+				dev_state = HLS_DN_DISABLE;
 				break;
 			default:
 				ret = -EINVAL;
-				break;
-			}
-
-			if (ret)
-				break;
-			/* fall through */
-		case HFI1_SNOOP_IOCGETLINKSTATE:
-		case HFI1_SNOOP_IOCGETLINKSTATE_EXTRA:
-			if (cmd == HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) {
-				memset(&link_info, 0, sizeof(link_info));
-				if (copy_from_user(&link_info,
-					(struct hfi1_link_info __user *)arg,
-					sizeof(link_info)))
-					ret = -EFAULT;
-				index = link_info.port_number;
-			} else {
-				ret = __get_user(index, (int __user *) arg);
-				if (ret !=  0)
-					break;
-			}
-
-			if (index > dd->num_pports - 1) {
-				ret = -EINVAL;
-				break;
-			}
-
-			ppd = &dd->pport[index];
-			if (!ppd) {
-				ret = -EINVAL;
-				break;
-			}
-			value = hfi1_ibphys_portstate(ppd);
-			value <<= 4;
-			value |= driver_lstate(ppd);
-
-			snoop_dbg("Link port | Link State: %d", value);
-
-			if ((cmd == HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) ||
-			    (cmd == HFI1_SNOOP_IOCSETLINKSTATE_EXTRA)) {
-				link_info.port_state = value;
-				link_info.node_guid = cpu_to_be64(ppd->guid);
-				link_info.link_speed_active =
-							ppd->link_speed_active;
-				link_info.link_width_active =
-							ppd->link_width_active;
-				if (copy_to_user(
-					(struct hfi1_link_info __user *)arg,
-					&link_info, sizeof(link_info)))
-					ret = -EFAULT;
-			} else {
-				ret = __put_user(value, (int __user *)arg);
+				goto done;
 			}
+			ret = set_link_state(ppd, dev_state);
 			break;
-
-		case HFI1_SNOOP_IOCCLEARQUEUE:
-			snoop_dbg("Clearing snoop queue");
-			drain_snoop_list(&dd->hfi1_snoop.queue);
+		case IB_PORT_ARMED:
+			ret = set_link_state(ppd, HLS_UP_ARMED);
+			if (!ret)
+				send_idle_sma(dd, SMA_IDLE_ARM);
 			break;
-
-		case HFI1_SNOOP_IOCCLEARFILTER:
-			snoop_dbg("Clearing filter");
-			if (dd->hfi1_snoop.filter_callback) {
-				/* Drain packets first */
-				drain_snoop_list(&dd->hfi1_snoop.queue);
-				dd->hfi1_snoop.filter_callback = NULL;
-			}
-			kfree(dd->hfi1_snoop.filter_value);
-			dd->hfi1_snoop.filter_value = NULL;
+		case IB_PORT_ACTIVE:
+			ret = set_link_state(ppd, HLS_UP_ACTIVE);
+			if (!ret)
+				send_idle_sma(dd, SMA_IDLE_ACTIVE);
+			break;
+		default:
+			ret = -EINVAL;
 			break;
+		}
 
-		case HFI1_SNOOP_IOCSETFILTER:
-			snoop_dbg("Setting filter");
-			/* just copy command structure */
-			argp = (unsigned long *)arg;
-			if (copy_from_user(&filter_cmd, (void __user *)argp,
-					     sizeof(filter_cmd))) {
+		if (ret)
+			break;
+		/* fall through */
+	case HFI1_SNOOP_IOCGETLINKSTATE:
+	case HFI1_SNOOP_IOCGETLINKSTATE_EXTRA:
+		if (cmd == HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) {
+			memset(&link_info, 0, sizeof(link_info));
+			if (copy_from_user(&link_info,
+					   (struct hfi1_link_info __user *)arg,
+					   sizeof(link_info)))
 				ret = -EFAULT;
+			index = link_info.port_number;
+		} else {
+			ret = __get_user(index, (int __user *)arg);
+			if (ret !=  0)
 				break;
-			}
-			if (filter_cmd.opcode >= HFI1_MAX_FILTERS) {
-				pr_alert("Invalid opcode in request\n");
-				ret = -EINVAL;
-				break;
-			}
+		}
 
-			snoop_dbg("Opcode %d Len %d Ptr %p",
-				   filter_cmd.opcode, filter_cmd.length,
-				   filter_cmd.value_ptr);
+		if (index > dd->num_pports - 1) {
+			ret = -EINVAL;
+			break;
+		}
 
-			filter_value = kcalloc(filter_cmd.length, sizeof(u8),
-					       GFP_KERNEL);
-			if (!filter_value) {
-				pr_alert("Not enough memory\n");
-				ret = -ENOMEM;
-				break;
-			}
-			/* copy remaining data from userspace */
-			if (copy_from_user((u8 *)filter_value,
-					(void __user *)filter_cmd.value_ptr,
-					filter_cmd.length)) {
-				kfree(filter_value);
+		ppd = &dd->pport[index];
+		if (!ppd) {
+			ret = -EINVAL;
+			break;
+		}
+		value = hfi1_ibphys_portstate(ppd);
+		value <<= 4;
+		value |= driver_lstate(ppd);
+
+		snoop_dbg("Link port | Link State: %d", value);
+
+		if ((cmd == HFI1_SNOOP_IOCGETLINKSTATE_EXTRA) ||
+		    (cmd == HFI1_SNOOP_IOCSETLINKSTATE_EXTRA)) {
+			link_info.port_state = value;
+			link_info.node_guid = cpu_to_be64(ppd->guid);
+			link_info.link_speed_active =
+						ppd->link_speed_active;
+			link_info.link_width_active =
+						ppd->link_width_active;
+			if (copy_to_user((struct hfi1_link_info __user *)arg,
+					 &link_info, sizeof(link_info)))
 				ret = -EFAULT;
-				break;
-			}
+		} else {
+			ret = __put_user(value, (int __user *)arg);
+		}
+		break;
+
+	case HFI1_SNOOP_IOCCLEARQUEUE:
+		snoop_dbg("Clearing snoop queue");
+		drain_snoop_list(&dd->hfi1_snoop.queue);
+		break;
+
+	case HFI1_SNOOP_IOCCLEARFILTER:
+		snoop_dbg("Clearing filter");
+		if (dd->hfi1_snoop.filter_callback) {
 			/* Drain packets first */
 			drain_snoop_list(&dd->hfi1_snoop.queue);
-			dd->hfi1_snoop.filter_callback =
-				hfi1_filters[filter_cmd.opcode].filter;
-			/* just in case we see back to back sets */
-			kfree(dd->hfi1_snoop.filter_value);
-			dd->hfi1_snoop.filter_value = filter_value;
+			dd->hfi1_snoop.filter_callback = NULL;
+		}
+		kfree(dd->hfi1_snoop.filter_value);
+		dd->hfi1_snoop.filter_value = NULL;
+		break;
 
+	case HFI1_SNOOP_IOCSETFILTER:
+		snoop_dbg("Setting filter");
+		/* just copy command structure */
+		argp = (unsigned long *)arg;
+		if (copy_from_user(&filter_cmd, (void __user *)argp,
+				   sizeof(filter_cmd))) {
+			ret = -EFAULT;
 			break;
-		case HFI1_SNOOP_IOCGETVERSION:
-			value = SNOOP_CAPTURE_VERSION;
-			snoop_dbg("Getting version: %d", value);
-			ret = __put_user(value, (int __user *)arg);
+		}
+		if (filter_cmd.opcode >= HFI1_MAX_FILTERS) {
+			pr_alert("Invalid opcode in request\n");
+			ret = -EINVAL;
 			break;
-		case HFI1_SNOOP_IOCSET_OPTS:
-			snoop_flags = 0;
-			ret = __get_user(value, (int __user *) arg);
-			if (ret != 0)
-				break;
+		}
 
-			snoop_dbg("Setting snoop option %d", value);
-			if (value & SNOOP_DROP_SEND)
-				snoop_flags |= SNOOP_DROP_SEND;
-			if (value & SNOOP_USE_METADATA)
-				snoop_flags |= SNOOP_USE_METADATA;
+		snoop_dbg("Opcode %d Len %d Ptr %p",
+			  filter_cmd.opcode, filter_cmd.length,
+			  filter_cmd.value_ptr);
+
+		filter_value = kcalloc(filter_cmd.length, sizeof(u8),
+				       GFP_KERNEL);
+		if (!filter_value) {
+			ret = -ENOMEM;
 			break;
-		default:
-			ret = -ENOTTY;
+		}
+		/* copy remaining data from userspace */
+		if (copy_from_user((u8 *)filter_value,
+				   (void __user *)filter_cmd.value_ptr,
+				   filter_cmd.length)) {
+			kfree(filter_value);
+			ret = -EFAULT;
 			break;
 		}
+		/* Drain packets first */
+		drain_snoop_list(&dd->hfi1_snoop.queue);
+		dd->hfi1_snoop.filter_callback =
+			hfi1_filters[filter_cmd.opcode].filter;
+		/* just in case we see back to back sets */
+		kfree(dd->hfi1_snoop.filter_value);
+		dd->hfi1_snoop.filter_value = filter_value;
+
+		break;
+	case HFI1_SNOOP_IOCGETVERSION:
+		value = SNOOP_CAPTURE_VERSION;
+		snoop_dbg("Getting version: %d", value);
+		ret = __put_user(value, (int __user *)arg);
+		break;
+	case HFI1_SNOOP_IOCSET_OPTS:
+		snoop_flags = 0;
+		ret = __get_user(value, (int __user *)arg);
+		if (ret != 0)
+			break;
+
+		snoop_dbg("Setting snoop option %d", value);
+		if (value & SNOOP_DROP_SEND)
+			snoop_flags |= SNOOP_DROP_SEND;
+		if (value & SNOOP_USE_METADATA)
+			snoop_flags |= SNOOP_USE_METADATA;
+		break;
+	default:
+		ret = -ENOTTY;
+		break;
 	}
 done:
 	spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags);