if: avoid interface destroy race

When we destroy an interface while the jail containing it is being
destroyed we risk seeing a race between if_vmove() and the destruction
code, which results in us trying to move a destroyed interface.

Protect against this by using the ifnet_detach_sxlock to also covert
if_vmove() (and not just detach).

PR:		262829
MFC after:	3 weeks
Differential Revision:	https://reviews.freebsd.org/D34704

2 files changed, 49 insertions, 2 deletions

diff --git a/sys/net/if.c b/sys/net/if.c
index ff7071cea364..bc0240035ea3 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -513,7 +513,9 @@ vnet_if_return(const void *unused __unused)
 	IFNET_WUNLOCK();
 
 	for (int j = 0; j < i; j++) {
+		sx_xlock(&ifnet_detach_sxlock);
 		if_vmove(pending[j], pending[j]->if_home_vnet);
+		sx_xunlock(&ifnet_detach_sxlock);
 	}
 
 	free(pending, M_IFNET);
@@ -1312,6 +1314,8 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, char *ifname, int jid)
 	bool found __diagused;
 	bool shutdown;
 
+	MPASS(ifindex_table[ifp->if_index].ife_ifnet == ifp);
+
 	/* Try to find the prison within our visibility. */
 	sx_slock(&allprison_lock);
 	pr = prison_find_child(td->td_ucred->cr_prison, jid);
@@ -1336,10 +1340,12 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, char *ifname, int jid)
 		prison_free(pr);
 		return (EEXIST);
 	}
+	sx_xlock(&ifnet_detach_sxlock);
 
 	/* Make sure the VNET is stable. */
 	shutdown = VNET_IS_SHUTTING_DOWN(ifp->if_vnet);
 	if (shutdown) {
+		sx_xunlock(&ifnet_detach_sxlock);
 		CURVNET_RESTORE();
 		prison_free(pr);
 		return (EBUSY);
@@ -1347,7 +1353,12 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, char *ifname, int jid)
 	CURVNET_RESTORE();
 
 	found = if_unlink_ifnet(ifp, true);
-	MPASS(found);
+	if (! found) {
+		sx_xunlock(&ifnet_detach_sxlock);
+		CURVNET_RESTORE();
+		prison_free(pr);
+		return (ENODEV);
+	}
 
 	/* Move the interface into the child jail/vnet. */
 	error = if_vmove(ifp, pr->pr_vnet);
@@ -1356,6 +1367,8 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, char *ifname, int jid)
 	if (error == 0)
 		sprintf(ifname, "%s", ifp->if_xname);
 
+	sx_xunlock(&ifnet_detach_sxlock);
+
 	prison_free(pr);
 	return (error);
 }
@@ -1406,7 +1419,9 @@ if_vmove_reclaim(struct thread *td, char *ifname, int jid)
 	/* Get interface back from child jail/vnet. */
 	found = if_unlink_ifnet(ifp, true);
 	MPASS(found);
+	sx_xlock(&ifnet_detach_sxlock);
 	error = if_vmove(ifp, vnet_dst);
+	sx_xunlock(&ifnet_detach_sxlock);
 	CURVNET_RESTORE();
 
 	/* Report the new if_xname back to the userland on success. */
@@ -2283,8 +2298,11 @@ ifunit_ref(const char *name)
 		    !(ifp->if_flags & IFF_DYING))
 			break;
 	}
-	if (ifp != NULL)
+	if (ifp != NULL) {
 		if_ref(ifp);
+		MPASS(ifindex_table[ifp->if_index].ife_ifnet == ifp);
+	}
+
 	NET_EPOCH_EXIT(et);
 	return (ifp);
 }
diff --git a/tests/sys/net/if_clone_test.sh b/tests/sys/net/if_clone_test.sh
index 0b50d741814f..60483a2da334 100755
--- a/tests/sys/net/if_clone_test.sh
+++ b/tests/sys/net/if_clone_test.sh
@@ -69,6 +69,34 @@ epair_up_stress_cleanup()
 	cleanup_ifaces
 }
 
+atf_test_case epair_destroy_race cleanup
+epair_destroy_race_head()
+{
+	atf_set "descr" "Race if_detach() and if_vmove()"
+	atf_set "require.user" "root"
+}
+epair_destroy_race_body()
+{
+	for i in `seq 1 10`
+	do
+		epair_a=$(ifconfig epair create)
+		echo $epair_a >> devices_to_cleanup
+		epair_b=${epair_a%a}b
+
+		jail -c vnet name="epair_destroy" nopersist path=/ \
+		  host.hostname="epair_destroy" vnet.interface="$epair_b" \
+		  command=sh -c "ifconfig $epair_b 192.0.2.1/24; sleep 0.1"&
+		pid=$!
+		ifconfig "$epair_a" destroy
+		wait $pid
+	done
+	true
+}
+epair_destroy_race_cleanup()
+{
+	cleanup_ifaces
+}
+
 atf_test_case epair_ipv6_up_stress cleanup
 epair_ipv6_up_stress_head()
 {
@@ -405,6 +433,7 @@ atf_init_test_cases()
 	atf_add_test_case epair_ipv6_up_stress
 	atf_add_test_case epair_stress
 	atf_add_test_case epair_up_stress
+	atf_add_test_case epair_destroy_race
 	atf_add_test_case faith_ipv6_up_stress
 	atf_add_test_case faith_stress
 	atf_add_test_case faith_up_stress