Date: Sat, 01 Feb 2020 19:26:48 +0000 From: "Kristof Provost" <kp@FreeBSD.org> To: "Gleb Smirnoff" <glebius@freebsd.org> Cc: "Ilja Van Sprundel" <ivansprundel@ioactive.com>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r357233 - head/sys/net Message-ID: <3F7A5204-4437-4845-8E40-0BC5D1CD0A5B@FreeBSD.org> In-Reply-To: <20200130163455.GH1268@FreeBSD.org> References: <202001282244.00SMiPrb077446@repo.freebsd.org> <20200130163455.GH1268@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 30 Jan 2020, at 16:34, Gleb Smirnoff wrote: > On Tue, Jan 28, 2020 at 10:44:25PM +0000, Kristof Provost wrote: > K> Author: kp > K> Date: Tue Jan 28 22:44:24 2020 > K> New Revision: 357233 > K> URL: https://svnweb.freebsd.org/changeset/base/357233 > K> > K> Log: > K> epair: Do not abuse params to register the second interface > K> > K> if_epair used the 'params' argument to pass a pointer to the b > interface > K> through if_clone_create(). > K> This pointer can be controlled by userspace, which means it could > be abused to > K> trigger a panic. While this requires PRIV_NET_IFCREATE > K> privileges those are assigned to vnet jails, which means that > vnet jails > K> could panic the system. > K> > K> Reported by: Ilja Van Sprundel <ivansprundel@ioactive.com> > ... > K> Modified: head/sys/net/if_clone.h > K> > ============================================================================== > K> --- head/sys/net/if_clone.h Tue Jan 28 21:46:59 2020 (r357232) > K> +++ head/sys/net/if_clone.h Tue Jan 28 22:44:24 2020 (r357233) > K> @@ -79,7 +79,8 @@ int if_clone_list(struct if_clonereq *); > K> struct if_clone *if_clone_findifc(struct ifnet *); > K> void if_clone_addgroup(struct ifnet *, struct if_clone *); > K> > K> -/* The below interface used only by epair(4). */ > K> +/* The below interfaces are used only by epair(4). */ > K> +void if_clone_addif(struct if_clone *, struct ifnet *); > K> int if_clone_destroyif(struct if_clone *, struct ifnet *); > > IMHO, makes sense to move all these declaration into if_epair.c > itself. > Yeah, that does make sense. One minor issue is that it turns out that if_clone_destroyif() isn’t just used by if_epair, but also by the wifi code. How does this look? diff --git a/sys/net/if_clone.c b/sys/net/if_clone.c index acc392ead16..452605b0464 100644 --- a/sys/net/if_clone.c +++ b/sys/net/if_clone.c @@ -106,6 +106,9 @@ static int ifc_simple_match(struct if_clone *, const char *); static int ifc_simple_create(struct if_clone *, char *, size_t, caddr_t); static int ifc_simple_destroy(struct if_clone *, struct ifnet *); +/* The below interface is used only by epair(4). */ +void if_clone_addif(struct if_clone *, struct ifnet *); + static struct mtx if_cloners_mtx; MTX_SYSINIT(if_cloners_lock, &if_cloners_mtx, "if_cloners lock", MTX_DEF); VNET_DEFINE_STATIC(int, if_cloners_count); diff --git a/sys/net/if_clone.h b/sys/net/if_clone.h index ed7d6f4d02d..c1ddf89c72d 100644 --- a/sys/net/if_clone.h +++ b/sys/net/if_clone.h @@ -79,8 +79,7 @@ int if_clone_list(struct if_clonereq *); struct if_clone *if_clone_findifc(struct ifnet *); void if_clone_addgroup(struct ifnet *, struct if_clone *); -/* The below interfaces are used only by epair(4). */ -void if_clone_addif(struct if_clone *, struct ifnet *); +/* The below interface is used only by epair(4) and ieee80211. */ int if_clone_destroyif(struct if_clone *, struct ifnet *); #endif /* _KERNEL */ diff --git a/sys/net/if_epair.c b/sys/net/if_epair.c index 376bdbe9117..7eff03b840f 100644 --- a/sys/net/if_epair.c +++ b/sys/net/if_epair.c @@ -94,6 +94,9 @@ SYSCTL_INT(_net_link_epair, OID_AUTO, epair_debug, CTLFLAG_RW, #define DPRINTF(fmt, arg...) #endif +/* if_clone private function, just for us. */ +extern void if_clone_addif(struct if_clone *, struct ifnet *); + static void epair_nh_sintr(struct mbuf *); static struct mbuf *epair_nh_m2cpuid(struct mbuf *, uintptr_t, u_int *); static void epair_nh_drainedcpu(u_int); Best regards, Kristof From owner-svn-src-all@freebsd.org Sat Feb 1 19:40:12 2020 Return-Path: <owner-svn-src-all@freebsd.org> 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 B8F6D229577; Sat, 1 Feb 2020 19:40:12 +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) server-signature RSA-PSS (4096 bits) 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 4894Dr3sl2z3xQl; Sat, 1 Feb 2020 19:40:12 +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 8001C3F1A; Sat, 1 Feb 2020 19:40:12 +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 011JeCZr015959; Sat, 1 Feb 2020 19:40:12 GMT (envelope-from kp@FreeBSD.org) Received: (from kp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 011JeCm1015958; Sat, 1 Feb 2020 19:40:12 GMT (envelope-from kp@FreeBSD.org) Message-Id: <202002011940.011JeCm1015958@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kp set sender to kp@FreeBSD.org using -f From: Kristof Provost <kp@FreeBSD.org> Date: Sat, 1 Feb 2020 19:40:12 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r357375 - head/tests/sys/net X-SVN-Group: head X-SVN-Commit-Author: kp X-SVN-Commit-Paths: head/tests/sys/net X-SVN-Commit-Revision: 357375 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.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" <svn-src-all.freebsd.org> List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-src-all>, <mailto:svn-src-all-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/svn-src-all/>; List-Post: <mailto:svn-src-all@freebsd.org> List-Help: <mailto:svn-src-all-request@freebsd.org?subject=help> List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-src-all>, <mailto:svn-src-all-request@freebsd.org?subject=subscribe> X-List-Received-Date: Sat, 01 Feb 2020 19:40:12 -0000 Author: kp Date: Sat Feb 1 19:40:11 2020 New Revision: 357375 URL: https://svnweb.freebsd.org/changeset/base/357375 Log: tests: epair: Don't fail if we load if_epair kldload() returns a positive integer when it loads a ko, so check that the return value is -1 to detect error cases, not that it's different from zero. MFC after: 3 days X-MFC-With: r357234 Modified: head/tests/sys/net/if_epair.c Modified: head/tests/sys/net/if_epair.c ============================================================================== --- head/tests/sys/net/if_epair.c Sat Feb 1 18:23:51 2020 (r357374) +++ head/tests/sys/net/if_epair.c Sat Feb 1 19:40:11 2020 (r357375) @@ -53,7 +53,7 @@ ATF_TC_BODY(params, tc) int s; s = kldload("if_epair"); - if (s != 0 && errno != EEXIST) + if (s == -1 && errno != EEXIST) atf_tc_fail("Failed to load if_epair"); s = socket(AF_INET, SOCK_DGRAM, 0);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3F7A5204-4437-4845-8E40-0BC5D1CD0A5B>