net: ipmr: rearrange and cleanup setsockopt

Take rtnl in the beginning unconditionally as most options already need
it (one exception - MRT_DONE, see the comment inside), make the
lock/unlock places central and move out the switch() local variables.

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

net/ipv4/ipmr.c

@@ -1276,38 +1276,45 @@ static void mrtsock_destruct(struct sock *sk)
  * MOSPF/PIM router set up we can clean this up.
  */
 
-int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsigned int optlen)
+int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval,
+			 unsigned int optlen)
 {
-	int ret, parent = 0;
+	struct net *net = sock_net(sk);
+	int val, ret = 0, parent = 0;
+	struct mr_table *mrt;
 	struct vifctl vif;
 	struct mfcctl mfc;
-	struct net *net = sock_net(sk);
+	u32 uval;
-	struct mr_table *mrt;
 
+	/* There's one exception to the lock - MRT_DONE which needs to unlock */
+	rtnl_lock();
 	if (sk->sk_type != SOCK_RAW ||
-	    inet_sk(sk)->inet_num != IPPROTO_IGMP)
+	    inet_sk(sk)->inet_num != IPPROTO_IGMP) {
-		return -EOPNOTSUPP;
+		ret = -EOPNOTSUPP;
+		goto out_unlock;
+	}
 
 	mrt = ipmr_get_table(net, raw_sk(sk)->ipmr_table ? : RT_TABLE_DEFAULT);
-	if (!mrt)
+	if (!mrt) {
-		return -ENOENT;
+		ret = -ENOENT;
-
+		goto out_unlock;
+	}
 	if (optname != MRT_INIT) {
 		if (sk != rcu_access_pointer(mrt->mroute_sk) &&
-		    !ns_capable(net->user_ns, CAP_NET_ADMIN))
+		    !ns_capable(net->user_ns, CAP_NET_ADMIN)) {
-			return -EACCES;
+			ret = -EACCES;
+			goto out_unlock;
+		}
 	}
 
 	switch (optname) {
 	case MRT_INIT:
 		if (optlen != sizeof(int))
-			return -EINVAL;
+			ret = -EINVAL;
-
+		if (rtnl_dereference(mrt->mroute_sk))
-		rtnl_lock();
+			ret = -EADDRINUSE;
-		if (rtnl_dereference(mrt->mroute_sk)) {
+		if (ret)
-			rtnl_unlock();
+			break;
-			return -EADDRINUSE;
-		}
 
 		ret = ip_ra_control(sk, 1, mrtsock_destruct);
 		if (ret == 0) {
@@ -1317,30 +1324,41 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
 						    NETCONFA_IFINDEX_ALL,
 						    net->ipv4.devconf_all);
 		}
-		rtnl_unlock();
+		break;
-		return ret;
 	case MRT_DONE:
-		if (sk != rcu_access_pointer(mrt->mroute_sk))
+		if (sk != rcu_access_pointer(mrt->mroute_sk)) {
-			return -EACCES;
+			ret = -EACCES;
-		return ip_ra_control(sk, 0, NULL);
+		} else {
+			/* We need to unlock here because mrtsock_destruct takes
+			 * care of rtnl itself and we can't change that due to
+			 * the IP_ROUTER_ALERT setsockopt which runs without it.
+			 */
+			rtnl_unlock();
+			ret = ip_ra_control(sk, 0, NULL);
+			goto out;
+		}
+		break;
 	case MRT_ADD_VIF:
 	case MRT_DEL_VIF:
-		if (optlen != sizeof(vif))
+		if (optlen != sizeof(vif)) {
-			return -EINVAL;
+			ret = -EINVAL;
-		if (copy_from_user(&vif, optval, sizeof(vif)))
+			break;
-			return -EFAULT;
+		}
-		if (vif.vifc_vifi >= MAXVIFS)
+		if (copy_from_user(&vif, optval, sizeof(vif))) {
-			return -ENFILE;
+			ret = -EFAULT;
-		rtnl_lock();
+			break;
+		}
+		if (vif.vifc_vifi >= MAXVIFS) {
+			ret = -ENFILE;
+			break;
+		}
 		if (optname == MRT_ADD_VIF) {
 			ret = vif_add(net, mrt, &vif,
 				      sk == rtnl_dereference(mrt->mroute_sk));
 		} else {
 			ret = vif_delete(mrt, vif.vifc_vifi, 0, NULL);
 		}
-		rtnl_unlock();
+		break;
-		return ret;
-
 	/* Manipulate the forwarding caches. These live
 	 * in a sort of kernel/user symbiosis.
 	 */
@@ -1349,82 +1367,87 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
 		parent = -1;
 	case MRT_ADD_MFC_PROXY:
 	case MRT_DEL_MFC_PROXY:
-		if (optlen != sizeof(mfc))
+		if (optlen != sizeof(mfc)) {
-			return -EINVAL;
+			ret = -EINVAL;
-		if (copy_from_user(&mfc, optval, sizeof(mfc)))
+			break;
-			return -EFAULT;
+		}
+		if (copy_from_user(&mfc, optval, sizeof(mfc))) {
+			ret = -EFAULT;
+			break;
+		}
 		if (parent == 0)
 			parent = mfc.mfcc_parent;
-		rtnl_lock();
 		if (optname == MRT_DEL_MFC || optname == MRT_DEL_MFC_PROXY)
 			ret = ipmr_mfc_delete(mrt, &mfc, parent);
 		else
 			ret = ipmr_mfc_add(net, mrt, &mfc,
 					   sk == rtnl_dereference(mrt->mroute_sk),
 					   parent);
-		rtnl_unlock();
+		break;
-		return ret;
 	/* Control PIM assert. */
 	case MRT_ASSERT:
-	{
+		if (optlen != sizeof(val)) {
-		int v;
+			ret = -EINVAL;
-		if (optlen != sizeof(v))
+			break;
-			return -EINVAL;
-		if (get_user(v, (int __user *)optval))
-			return -EFAULT;
-		mrt->mroute_do_assert = v;
-		return 0;
-	}
-	case MRT_PIM:
-	{
-		int v;
-
-		if (!pimsm_enabled())
-			return -ENOPROTOOPT;
-		if (optlen != sizeof(v))
-			return -EINVAL;
-		if (get_user(v, (int __user *)optval))
-			return -EFAULT;
-		v = !!v;
-
-		rtnl_lock();
-		ret = 0;
-		if (v != mrt->mroute_do_pim) {
-			mrt->mroute_do_pim = v;
-			mrt->mroute_do_assert = v;
 		}
-		rtnl_unlock();
+		if (get_user(val, (int __user *)optval)) {
-		return ret;
+			ret = -EFAULT;
-	}
+			break;
+		}
+		mrt->mroute_do_assert = val;
+		break;
+	case MRT_PIM:
+		if (!pimsm_enabled()) {
+			ret = -ENOPROTOOPT;
+			break;
+		}
+		if (optlen != sizeof(val)) {
+			ret = -EINVAL;
+			break;
+		}
+		if (get_user(val, (int __user *)optval)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		val = !!val;
+		if (val != mrt->mroute_do_pim) {
+			mrt->mroute_do_pim = val;
+			mrt->mroute_do_assert = val;
+		}
+		break;
 	case MRT_TABLE:
-	{
+		if (!IS_BUILTIN(CONFIG_IP_MROUTE_MULTIPLE_TABLES)) {
-		u32 v;
+			ret = -ENOPROTOOPT;
+			break;
+		}
+		if (optlen != sizeof(uval)) {
+			ret = -EINVAL;
+			break;
+		}
+		if (get_user(uval, (u32 __user *)optval)) {
+			ret = -EFAULT;
+			break;
+		}
 
-		if (!IS_BUILTIN(CONFIG_IP_MROUTE_MULTIPLE_TABLES))
-			return -ENOPROTOOPT;
-		if (optlen != sizeof(u32))
-			return -EINVAL;
-		if (get_user(v, (u32 __user *)optval))
-			return -EFAULT;
-
-		rtnl_lock();
-		ret = 0;
 		if (sk == rtnl_dereference(mrt->mroute_sk)) {
 			ret = -EBUSY;
 		} else {
-			mrt = ipmr_new_table(net, v);
+			mrt = ipmr_new_table(net, uval);
 			if (IS_ERR(mrt))
 				ret = PTR_ERR(mrt);
 			else
-				raw_sk(sk)->ipmr_table = v;
+				raw_sk(sk)->ipmr_table = uval;
 		}
-		rtnl_unlock();
+		break;
-		return ret;
-	}
 	/* Spurious command, or MRT_VERSION which you cannot set. */
 	default:
-		return -ENOPROTOOPT;
+		ret = -ENOPROTOOPT;
 	}
+out_unlock:
+	rtnl_unlock();
+out:
+	return ret;
 }
 
 /* Getsock opt support for the multicast routing system. */