diff options
author | Kristof Provost <kp@FreeBSD.org> | 2022-03-27 18:23:25 +0000 |
---|---|---|
committer | Kristof Provost <kp@FreeBSD.org> | 2022-05-27 16:25:10 +0000 |
commit | 4dfd3ffc4488e5e2662cdc40deec17d82432da0b (patch) | |
tree | 4f5f6ecf536faa3a83c488cf39903e30db5e6fa4 | |
parent | 8fab6c48493177662a9d34694cf23a0be719bbf1 (diff) | |
download | src-4dfd3ffc4488e5e2662cdc40deec17d82432da0b.tar.gz src-4dfd3ffc4488e5e2662cdc40deec17d82432da0b.zip |
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)
-rw-r--r-- | sys/net/if.c | 22 | ||||
-rwxr-xr-x | 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 |