Make the in-kernel logic for the SIOCSIFVNET, SIOCSIFRVNET ioctls

(ifconfig ifN (-)vnet <jname|jid>) work correctly.

Move vi_if_move to if.c and split it up into two functions(*),
one for each ioctl.

In the reclaim case, correctly set the vnet before calling if_vmove.

Instead of silently allowing a move of an interface from the current
vnet to the current vnet, return an error. (*)

There is some duplicate interface name checking before actually moving
the interface between network stacks without locking and thus race
prone. Ideally if_vmove will correctly and automagically handle these
in the future.

Suggested by:	rwatson (*)
Approved by:	re (kib)

3 files changed, 90 insertions, 58 deletions

diff --git a/sys/kern/kern_vimage.c b/sys/kern/kern_vimage.c
index fa2d1f4..9c86b75 100644
--- a/sys/kern/kern_vimage.c
+++ b/sys/kern/kern_vimage.c
@@ -68,61 +68,6 @@ struct sx		vnet_sxlock;
 struct vnet_list_head vnet_head;
 struct vnet *vnet0;
 
-/*
- * Move an ifnet to or from another vnet, specified by the jail id.
- */
-int
-vi_if_move(struct thread *td, struct ifnet *ifp, char *ifname, int jid)
-{
-	struct ifnet *t_ifp;
-	struct prison *pr;
-	struct vnet *new_vnet;
-	int error;
-
-	sx_slock(&allprison_lock);
-	pr = prison_find_child(td->td_ucred->cr_prison, jid);
-	sx_sunlock(&allprison_lock);
-	if (pr == NULL)
-		return (ENXIO);
-	prison_hold_locked(pr);
-	mtx_unlock(&pr->pr_mtx);
-	if (ifp != NULL) {
-		/* SIOCSIFVNET */
-		new_vnet = pr->pr_vnet;
-	} else {
-		/* SIOCSIFRVNET */
-		new_vnet = TD_TO_VNET(td);
-		CURVNET_SET(pr->pr_vnet);
-		ifp = ifunit(ifname);
-		CURVNET_RESTORE();
-		if (ifp == NULL) {
-			prison_free(pr);
-			return (ENXIO);
-		}
-	}
-
-	error = 0;
-	if (new_vnet != ifp->if_vnet) {
-		/*
-		 * Check for naming clashes in target vnet.  Not locked so races
-		 * are possible.
-		 */
-		CURVNET_SET_QUIET(new_vnet);
-		t_ifp = ifunit(ifname);
-		CURVNET_RESTORE();
-		if (t_ifp != NULL)
-			error = EEXIST;
-		else {
-			/* Detach from curvnet and attach to new_vnet. */
-			if_vmove(ifp, new_vnet);
-
-			/* Report the new if_xname back to the userland */
-			sprintf(ifname, "%s", ifp->if_xname);
-		}
-	}
-	prison_free(pr);
-	return (error);
-}
 
 struct vnet *
 vnet_alloc(void)
diff --git a/sys/net/if.c b/sys/net/if.c
index 3916675..02796cb 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -894,6 +894,94 @@ if_vmove(struct ifnet *ifp, struct vnet *new_vnet)
 
 	CURVNET_RESTORE();
 }
+
+/*
+ * Move an ifnet to or from another child prison/vnet, specified by the jail id.
+ */
+static int
+if_vmove_loan(struct thread *td, struct ifnet *ifp, char *ifname, int jid)
+{
+	struct prison *pr;
+	struct ifnet *difp;
+
+	/* Try to find the prison within our visibility. */
+	sx_slock(&allprison_lock);
+	pr = prison_find_child(td->td_ucred->cr_prison, jid);
+	sx_sunlock(&allprison_lock);
+	if (pr == NULL)
+		return (ENXIO);
+	prison_hold_locked(pr);
+	mtx_unlock(&pr->pr_mtx);
+
+	/* Do not try to move the iface from and to the same prison. */
+	if (pr->pr_vnet == ifp->if_vnet) {
+		prison_free(pr);
+		return (EEXIST);
+	}
+
+	/* Make sure the named iface does not exists in the dst. prison/vnet. */
+	/* XXX Lock interfaces to avoid races. */
+	CURVNET_SET(pr->pr_vnet);
+	difp = ifunit(ifname);
+	CURVNET_RESTORE();
+	if (difp != NULL) {
+		prison_free(pr);
+		return (EEXIST);
+	}
+
+	/* Move the interface into the child jail/vnet. */
+	if_vmove(ifp, pr->pr_vnet);
+
+	/* Report the new if_xname back to the userland. */
+	sprintf(ifname, "%s", ifp->if_xname);
+
+	prison_free(pr);
+	return (0);
+}
+
+static int
+if_vmove_reclaim(struct thread *td, char *ifname, int jid)
+{
+	struct prison *pr;
+	struct vnet *vnet_dst;
+	struct ifnet *ifp;
+
+	/* Try to find the prison within our visibility. */
+	sx_slock(&allprison_lock);
+	pr = prison_find_child(td->td_ucred->cr_prison, jid);
+	sx_sunlock(&allprison_lock);
+	if (pr == NULL)
+		return (ENXIO);
+	prison_hold_locked(pr);
+	mtx_unlock(&pr->pr_mtx);
+
+	/* Make sure the named iface exists in the source prison/vnet. */
+	CURVNET_SET(pr->pr_vnet);
+	ifp = ifunit(ifname);		/* XXX Lock to avoid races. */
+	if (ifp == NULL) {
+		CURVNET_RESTORE();
+		prison_free(pr);
+		return (ENXIO);
+	}
+
+	/* Do not try to move the iface from and to the same prison. */
+	vnet_dst = TD_TO_VNET(td);
+	if (vnet_dst == ifp->if_vnet) {
+		CURVNET_RESTORE();
+		prison_free(pr);
+		return (EEXIST);
+	}
+
+	/* Get interface back from child jail/vnet. */
+	if_vmove(ifp, vnet_dst);
+	CURVNET_RESTORE();
+
+	/* Report the new if_xname back to the userland. */
+	sprintf(ifname, "%s", ifp->if_xname);
+
+	prison_free(pr);
+	return (0);
+}
 #endif /* VIMAGE */
 
 /*
@@ -1990,7 +2078,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t data, struct thread *td)
 		error = priv_check(td, PRIV_NET_SETIFVNET);
 		if (error)
 			return (error);
-		error = vi_if_move(td, ifp, ifr->ifr_name, ifr->ifr_jid);
+		error = if_vmove_loan(td, ifp, ifr->ifr_name, ifr->ifr_jid);
 		break;
 #endif
 
@@ -2184,7 +2272,7 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct thread *td)
 		error = priv_check(td, PRIV_NET_SETIFVNET);
 		if (error)
 			return (error);
-		return (vi_if_move(td, NULL, ifr->ifr_name, ifr->ifr_jid));
+		return (if_vmove_reclaim(td, ifr->ifr_name, ifr->ifr_jid));
 #endif
 	case SIOCIFCREATE:
 	case SIOCIFCREATE2:
diff --git a/sys/sys/vimage.h b/sys/sys/vimage.h
index 9dd0e5f..32084b7 100644
--- a/sys/sys/vimage.h
+++ b/sys/sys/vimage.h
@@ -67,7 +67,6 @@ struct vnet {
 
 struct vnet;
 struct ifnet;
-int	vi_if_move(struct thread *, struct ifnet *, char *, int);
 struct vnet *vnet_alloc(void);
 void	vnet_destroy(struct vnet *);
 void	vnet_foreach(void (*vnet_foreach_fn)(struct vnet *, void *),