The branch stable/13 has been updated by kp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=4dfd3ffc4488e5e2662cdc40deec17d82432da0b
commit 4dfd3ffc4488e5e2662cdc40deec17d82432da0b
Author: Kristof Provost <k...@freebsd.org>
AuthorDate: 2022-03-27 18:23:25 +0000
Commit: Kristof Provost <k...@freebsd.org>
CommitDate: 2022-05-27 16:25:10 +0000
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
(cherry picked from commit 868bf82153e8ff22f09a8860c872149e0fb6bdef)
---
sys/net/if.c | 22 ++++++++++++++++++++--
tests/sys/net/if_clone_test.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/sys/net/if.c b/sys/net/if.c
index d4871ccbc1f7..091e9e64b99f 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -548,7 +548,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);
@@ -1393,6 +1395,8 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, char
*ifname, int jid)
bool found;
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);
@@ -1417,10 +1421,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);
@@ -1428,7 +1434,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);
@@ -1437,6 +1448,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);
}
@@ -1487,7 +1500,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. */
@@ -2397,8 +2412,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
