if: Protect V_ifnet in vnet_if_return()

When we terminate a vnet (i.e. jail) we move interfaces back to their home
vnet. We need to protect our access to the V_ifnet CK_LIST.

We could enter NET_EPOCH, but if_detach_internal() (called from if_vmove())
waits for net epoch callback completion. That's not possible from NET_EPOCH.
Instead, we take the IFNET_WLOCK, build a list of the interfaces that need to
move and, once we've released the lock, move them back to their home vnet.

We cannot hold the IFNET_WLOCK() during if_vmove(), because that results in a
LOR between ifnet_sx, in_multi_sx and iflib ctx lock.

Separate out moving the ifp into or out of V_ifnet, so we can hold the lock as
we do the list manipulation, but do not hold it as we if_vmove().

Reviewed by:	melifaro
MFC after:	2 weeks
Sponsored by:	Modirum MDPay
Differential Revision:	https://reviews.freebsd.org/D27279
This commit is contained in:
Kristof Provost 2020-11-25 15:07:22 +00:00
parent 3063e1e56b
commit a779388f8b

View file

@ -275,6 +275,8 @@ static void if_delgroups(struct ifnet *);
static void if_attach_internal(struct ifnet *, int, struct if_clone *);
static int if_detach_internal(struct ifnet *, int, struct if_clone **);
static void if_siocaddmulti(void *, int);
static void if_link_ifnet(struct ifnet *);
static bool if_unlink_ifnet(struct ifnet *, bool);
#ifdef VIMAGE
static int if_vmove(struct ifnet *, struct vnet *);
#endif
@ -467,16 +469,82 @@ vnet_if_uninit(const void *unused __unused)
VNET_SYSUNINIT(vnet_if_uninit, SI_SUB_INIT_IF, SI_ORDER_FIRST,
vnet_if_uninit, NULL);
static void
if_link_ifnet(struct ifnet *ifp)
{
IFNET_WLOCK();
CK_STAILQ_INSERT_TAIL(&V_ifnet, ifp, if_link);
#ifdef VIMAGE
curvnet->vnet_ifcnt++;
#endif
IFNET_WUNLOCK();
}
static bool
if_unlink_ifnet(struct ifnet *ifp, bool vmove)
{
struct ifnet *iter;
int found = 0;
IFNET_WLOCK();
CK_STAILQ_FOREACH(iter, &V_ifnet, if_link)
if (iter == ifp) {
CK_STAILQ_REMOVE(&V_ifnet, ifp, ifnet, if_link);
if (!vmove)
ifp->if_flags |= IFF_DYING;
found = 1;
break;
}
#ifdef VIMAGE
curvnet->vnet_ifcnt--;
#endif
IFNET_WUNLOCK();
return (found);
}
static void
vnet_if_return(const void *unused __unused)
{
struct ifnet *ifp, *nifp;
struct ifnet **pending;
int found, i;
i = 0;
/*
* We need to protect our access to the V_ifnet tailq. Ordinarily we'd
* enter NET_EPOCH, but that's not possible, because if_vmove() calls
* if_detach_internal(), which waits for NET_EPOCH callbacks to
* complete. We can't do that from within NET_EPOCH.
*
* However, we can also use the IFNET_xLOCK, which is the V_ifnet
* read/write lock. We cannot hold the lock as we call if_vmove()
* though, as that presents LOR w.r.t ifnet_sx, in_multi_sx and iflib
* ctx lock.
*/
IFNET_WLOCK();
pending = malloc(sizeof(struct ifnet *) * curvnet->vnet_ifcnt,
M_IFNET, M_WAITOK | M_ZERO);
/* Return all inherited interfaces to their parent vnets. */
CK_STAILQ_FOREACH_SAFE(ifp, &V_ifnet, if_link, nifp) {
if (ifp->if_home_vnet != ifp->if_vnet)
if_vmove(ifp, ifp->if_home_vnet);
if (ifp->if_home_vnet != ifp->if_vnet) {
found = if_unlink_ifnet(ifp, true);
MPASS(found);
pending[i++] = ifp;
}
}
IFNET_WUNLOCK();
for (int j = 0; j < i; j++) {
if_vmove(pending[j], pending[j]->if_home_vnet);
}
free(pending, M_IFNET);
}
VNET_SYSUNINIT(vnet_if_return, SI_SUB_VNET_DONE, SI_ORDER_ANY,
vnet_if_return, NULL);
@ -906,12 +974,7 @@ if_attach_internal(struct ifnet *ifp, int vmove, struct if_clone *ifc)
}
#endif
IFNET_WLOCK();
CK_STAILQ_INSERT_TAIL(&V_ifnet, ifp, if_link);
#ifdef VIMAGE
curvnet->vnet_ifcnt++;
#endif
IFNET_WUNLOCK();
if_link_ifnet(ifp);
if (domain_init_status >= 2)
if_attachdomain1(ifp);
@ -1049,9 +1112,12 @@ if_purgemaddrs(struct ifnet *ifp)
void
if_detach(struct ifnet *ifp)
{
bool found;
CURVNET_SET_QUIET(ifp->if_vnet);
if_detach_internal(ifp, 0, NULL);
found = if_unlink_ifnet(ifp, false);
if (found)
if_detach_internal(ifp, 0, NULL);
CURVNET_RESTORE();
}
@ -1071,46 +1137,16 @@ if_detach_internal(struct ifnet *ifp, int vmove, struct if_clone **ifcp)
struct ifaddr *ifa;
int i;
struct domain *dp;
struct ifnet *iter;
int found = 0;
#ifdef VIMAGE
bool shutdown;
shutdown = VNET_IS_SHUTTING_DOWN(ifp->if_vnet);
#endif
IFNET_WLOCK();
CK_STAILQ_FOREACH(iter, &V_ifnet, if_link)
if (iter == ifp) {
CK_STAILQ_REMOVE(&V_ifnet, ifp, ifnet, if_link);
if (!vmove)
ifp->if_flags |= IFF_DYING;
found = 1;
break;
}
IFNET_WUNLOCK();
if (!found) {
/*
* While we would want to panic here, we cannot
* guarantee that the interface is indeed still on
* the list given we don't hold locks all the way.
*/
return (ENOENT);
#if 0
if (vmove)
panic("%s: ifp=%p not on the ifnet tailq %p",
__func__, ifp, &V_ifnet);
else
return; /* XXX this should panic as well? */
#endif
}
/*
* At this point we know the interface still was on the ifnet list
* and we removed it so we are in a stable state.
*/
#ifdef VIMAGE
curvnet->vnet_ifcnt--;
#endif
epoch_wait_preempt(net_epoch_preempt);
/*
@ -1340,6 +1376,7 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, char *ifname, int jid)
struct prison *pr;
struct ifnet *difp;
int error;
bool found;
bool shutdown;
/* Try to find the prison within our visibility. */
@ -1376,6 +1413,9 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, char *ifname, int jid)
}
CURVNET_RESTORE();
found = if_unlink_ifnet(ifp, true);
MPASS(found);
/* Move the interface into the child jail/vnet. */
error = if_vmove(ifp, pr->pr_vnet);
@ -1393,7 +1433,7 @@ if_vmove_reclaim(struct thread *td, char *ifname, int jid)
struct prison *pr;
struct vnet *vnet_dst;
struct ifnet *ifp;
int error;
int error, found;
bool shutdown;
/* Try to find the prison within our visibility. */
@ -1431,6 +1471,8 @@ 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);
error = if_vmove(ifp, vnet_dst);
CURVNET_RESTORE();