Author: kp Date: Wed Nov 25 15:07:22 2020 New Revision: 368020 URL: https://svnweb.freebsd.org/changeset/base/368020
Log: 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 Modified: head/sys/net/if.c Modified: head/sys/net/if.c ============================================================================== --- head/sys/net/if.c Wed Nov 25 14:26:13 2020 (r368019) +++ head/sys/net/if.c Wed Nov 25 15:07:22 2020 (r368020) @@ -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 @@ -468,15 +470,81 @@ VNET_SYSUNINIT(vnet_if_uninit, SI_SUB_INIT_IF, SI_ORDE 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, struc } #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, struc 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, ch 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, ch } 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 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 } /* Get interface back from child jail/vnet. */ + found = if_unlink_ifnet(ifp, true); + MPASS(found); error = if_vmove(ifp, vnet_dst); CURVNET_RESTORE(); _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
