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
This commit is contained in:
parent
b29fb6cffd
commit
868bf82153
22
sys/net/if.c
22
sys/net/if.c
@ -513,7 +513,9 @@ vnet_if_return(const void *unused __unused)
|
|||||||
IFNET_WUNLOCK();
|
IFNET_WUNLOCK();
|
||||||
|
|
||||||
for (int j = 0; j < i; j++) {
|
for (int j = 0; j < i; j++) {
|
||||||
|
sx_xlock(&ifnet_detach_sxlock);
|
||||||
if_vmove(pending[j], pending[j]->if_home_vnet);
|
if_vmove(pending[j], pending[j]->if_home_vnet);
|
||||||
|
sx_xunlock(&ifnet_detach_sxlock);
|
||||||
}
|
}
|
||||||
|
|
||||||
free(pending, M_IFNET);
|
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 found __diagused;
|
||||||
bool shutdown;
|
bool shutdown;
|
||||||
|
|
||||||
|
MPASS(ifindex_table[ifp->if_index].ife_ifnet == ifp);
|
||||||
|
|
||||||
/* Try to find the prison within our visibility. */
|
/* Try to find the prison within our visibility. */
|
||||||
sx_slock(&allprison_lock);
|
sx_slock(&allprison_lock);
|
||||||
pr = prison_find_child(td->td_ucred->cr_prison, jid);
|
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);
|
prison_free(pr);
|
||||||
return (EEXIST);
|
return (EEXIST);
|
||||||
}
|
}
|
||||||
|
sx_xlock(&ifnet_detach_sxlock);
|
||||||
|
|
||||||
/* Make sure the VNET is stable. */
|
/* Make sure the VNET is stable. */
|
||||||
shutdown = VNET_IS_SHUTTING_DOWN(ifp->if_vnet);
|
shutdown = VNET_IS_SHUTTING_DOWN(ifp->if_vnet);
|
||||||
if (shutdown) {
|
if (shutdown) {
|
||||||
|
sx_xunlock(&ifnet_detach_sxlock);
|
||||||
CURVNET_RESTORE();
|
CURVNET_RESTORE();
|
||||||
prison_free(pr);
|
prison_free(pr);
|
||||||
return (EBUSY);
|
return (EBUSY);
|
||||||
@ -1347,7 +1353,12 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, char *ifname, int jid)
|
|||||||
CURVNET_RESTORE();
|
CURVNET_RESTORE();
|
||||||
|
|
||||||
found = if_unlink_ifnet(ifp, true);
|
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. */
|
/* Move the interface into the child jail/vnet. */
|
||||||
error = if_vmove(ifp, pr->pr_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)
|
if (error == 0)
|
||||||
sprintf(ifname, "%s", ifp->if_xname);
|
sprintf(ifname, "%s", ifp->if_xname);
|
||||||
|
|
||||||
|
sx_xunlock(&ifnet_detach_sxlock);
|
||||||
|
|
||||||
prison_free(pr);
|
prison_free(pr);
|
||||||
return (error);
|
return (error);
|
||||||
}
|
}
|
||||||
@ -1406,7 +1419,9 @@ if_vmove_reclaim(struct thread *td, char *ifname, int jid)
|
|||||||
/* Get interface back from child jail/vnet. */
|
/* Get interface back from child jail/vnet. */
|
||||||
found = if_unlink_ifnet(ifp, true);
|
found = if_unlink_ifnet(ifp, true);
|
||||||
MPASS(found);
|
MPASS(found);
|
||||||
|
sx_xlock(&ifnet_detach_sxlock);
|
||||||
error = if_vmove(ifp, vnet_dst);
|
error = if_vmove(ifp, vnet_dst);
|
||||||
|
sx_xunlock(&ifnet_detach_sxlock);
|
||||||
CURVNET_RESTORE();
|
CURVNET_RESTORE();
|
||||||
|
|
||||||
/* Report the new if_xname back to the userland on success. */
|
/* 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))
|
!(ifp->if_flags & IFF_DYING))
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
if (ifp != NULL)
|
if (ifp != NULL) {
|
||||||
if_ref(ifp);
|
if_ref(ifp);
|
||||||
|
MPASS(ifindex_table[ifp->if_index].ife_ifnet == ifp);
|
||||||
|
}
|
||||||
|
|
||||||
NET_EPOCH_EXIT(et);
|
NET_EPOCH_EXIT(et);
|
||||||
return (ifp);
|
return (ifp);
|
||||||
}
|
}
|
||||||
|
@ -69,6 +69,34 @@ epair_up_stress_cleanup()
|
|||||||
cleanup_ifaces
|
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
|
atf_test_case epair_ipv6_up_stress cleanup
|
||||||
epair_ipv6_up_stress_head()
|
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_ipv6_up_stress
|
||||||
atf_add_test_case epair_stress
|
atf_add_test_case epair_stress
|
||||||
atf_add_test_case epair_up_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_ipv6_up_stress
|
||||||
atf_add_test_case faith_stress
|
atf_add_test_case faith_stress
|
||||||
atf_add_test_case faith_up_stress
|
atf_add_test_case faith_up_stress
|
||||||
|
Loading…
Reference in New Issue
Block a user