git: 4dfd3ffc4488 - stable/13 - if: avoid interface destroy race
Date: Fri, 27 May 2022 16:37:17 UTC
The branch stable/13 has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=4dfd3ffc4488e5e2662cdc40deec17d82432da0b commit 4dfd3ffc4488e5e2662cdc40deec17d82432da0b Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2022-03-27 18:23:25 +0000 Commit: Kristof Provost <kp@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