From owner-svn-src-all@freebsd.org Tue Dec 22 15:03:46 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E20F2A4EEB6; Tue, 22 Dec 2015 15:03:46 +0000 (UTC) (envelope-from bz@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 mx1.freebsd.org (Postfix) with ESMTPS id 98A2A1443; Tue, 22 Dec 2015 15:03:46 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id tBMF3jBW045993; Tue, 22 Dec 2015 15:03:45 GMT (envelope-from bz@FreeBSD.org) Received: (from bz@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id tBMF3jQ8045992; Tue, 22 Dec 2015 15:03:45 GMT (envelope-from bz@FreeBSD.org) Message-Id: <201512221503.tBMF3jQ8045992@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: bz set sender to bz@FreeBSD.org using -f From: "Bjoern A. Zeeb" Date: Tue, 22 Dec 2015 15:03:45 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r292604 - head/sys/net X-SVN-Group: head 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.20 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: Tue, 22 Dec 2015 15:03:47 -0000 Author: bz Date: Tue Dec 22 15:03:45 2015 New Revision: 292604 URL: https://svnweb.freebsd.org/changeset/base/292604 Log: If vnets are torn down while ifconfig runs an ioctl to say, destroy an epair(4), we may hit if_detach_internal() without holding a lock and by the time we aquire it the interface might be gone. We should not panic() in this case as it is our fault for not holding the lock all the way. It is not ideal to return silently without error to user space, but other callers will all ignore the return values so do not change the entire KPI for little benefit for now. The ifp will be dealt with one way or another still. Sponsored by: The FreeBSD Foundation MFC after: 2 weeks Reviewed by: gnn Differential Revision: https://reviews.freebsd.org/D4529 Modified: head/sys/net/if.c Modified: head/sys/net/if.c ============================================================================== --- head/sys/net/if.c Tue Dec 22 15:00:04 2015 (r292603) +++ head/sys/net/if.c Tue Dec 22 15:03:45 2015 (r292604) @@ -173,7 +173,7 @@ static int if_getgroup(struct ifgroupreq static int if_getgroupmembers(struct ifgroupreq *); static void if_delgroups(struct ifnet *); static void if_attach_internal(struct ifnet *, int, struct if_clone *); -static void if_detach_internal(struct ifnet *, int, struct if_clone **); +static int if_detach_internal(struct ifnet *, int, struct if_clone **); #ifdef INET6 /* @@ -884,7 +884,7 @@ if_detach(struct ifnet *ifp) CURVNET_RESTORE(); } -static void +static int if_detach_internal(struct ifnet *ifp, int vmove, struct if_clone **ifcp) { struct ifaddr *ifa; @@ -906,11 +906,19 @@ if_detach_internal(struct ifnet *ifp, in #endif 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 } /* Check if this is a cloned interface or not. */ @@ -993,6 +1001,8 @@ if_detach_internal(struct ifnet *ifp, in (*dp->dom_ifdetach)(ifp, ifp->if_afdata[dp->dom_family]); } + + return (0); } #ifdef VIMAGE @@ -1007,12 +1017,16 @@ void if_vmove(struct ifnet *ifp, struct vnet *new_vnet) { struct if_clone *ifc; + int rc; /* * Detach from current vnet, but preserve LLADDR info, do not * mark as dead etc. so that the ifnet can be reattached later. + * If we cannot find it, we lost the race to someone else. */ - if_detach_internal(ifp, 1, &ifc); + rc = if_detach_internal(ifp, 1, &ifc); + if (rc != 0) + return; /* * Unlink the ifnet from ifindex_table[] in current vnet, and shrink