From owner-svn-src-all@freebsd.org Fri Dec 11 15:39:23 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 8BE684B2D0F; Fri, 11 Dec 2020 15:39:23 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Csw233Chvz4Vwv; Fri, 11 Dec 2020 15:39:23 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 5CB1D53DB; Fri, 11 Dec 2020 15:39:23 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 0BBFdNbH050355; Fri, 11 Dec 2020 15:39:23 GMT (envelope-from kp@FreeBSD.org) Received: (from kp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 0BBFdNWV050354; Fri, 11 Dec 2020 15:39:23 GMT (envelope-from kp@FreeBSD.org) Message-Id: <202012111539.0BBFdNWV050354@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kp set sender to kp@FreeBSD.org using -f From: Kristof Provost Date: Fri, 11 Dec 2020 15:39:23 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r368554 - stable/12/sys/net X-SVN-Group: stable-12 X-SVN-Commit-Author: kp X-SVN-Commit-Paths: stable/12/sys/net X-SVN-Commit-Revision: 368554 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Dec 2020 15:39:23 -0000 Author: kp Date: Fri Dec 11 15:39:22 2020 New Revision: 368554 URL: https://svnweb.freebsd.org/changeset/base/368554 Log: MFC r368020, r368025: 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(). if: Fix non-VIMAGE build if_link_ifnet() and if_unlink_ifnet() are needed even when VIMAGE is not enabled. Sponsored by: Modirum MDPay Modified: stable/12/sys/net/if.c Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/net/if.c ============================================================================== --- stable/12/sys/net/if.c Fri Dec 11 14:32:42 2020 (r368553) +++ stable/12/sys/net/if.c Fri Dec 11 15:39:22 2020 (r368554) @@ -274,6 +274,8 @@ static int if_getgroupmembers(struct ifgroupreq *); 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_link_ifnet(struct ifnet *); +static bool if_unlink_ifnet(struct ifnet *, bool); #ifdef VIMAGE static void if_vmove(struct ifnet *, struct vnet *); #endif @@ -472,17 +474,85 @@ vnet_if_uninit(const void *unused __unused) } VNET_SYSUNINIT(vnet_if_uninit, SI_SUB_INIT_IF, SI_ORDER_FIRST, vnet_if_uninit, NULL); +#endif 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); +} + +#ifdef VIMAGE +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); @@ -890,12 +960,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); @@ -1033,9 +1098,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(); } @@ -1055,47 +1123,17 @@ 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 int shutdown; shutdown = (ifp->if_vnet->vnet_state > SI_SUB_VNET && ifp->if_vnet->vnet_state < SI_SUB_VNET_DONE) ? 1 : 0; #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); /* @@ -1322,6 +1360,7 @@ if_vmove_loan(struct thread *td, struct ifnet *ifp, ch struct prison *pr; struct ifnet *difp; int shutdown; + bool found; /* Try to find the prison within our visibility. */ sx_slock(&allprison_lock); @@ -1358,6 +1397,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. */ if_vmove(ifp, pr->pr_vnet); @@ -1374,7 +1416,8 @@ if_vmove_reclaim(struct thread *td, char *ifname, int struct prison *pr; struct vnet *vnet_dst; struct ifnet *ifp; - int shutdown; + int shutdown; + bool found; /* Try to find the prison within our visibility. */ sx_slock(&allprison_lock); @@ -1412,6 +1455,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); if_vmove(ifp, vnet_dst); CURVNET_RESTORE();