Skip site navigation (1)Skip section navigation (2)
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-head@freebsd.org  Sat Feb  1 19:40:12 2020
Return-Path: <owner-svn-src-head@freebsd.org>
Delivered-To: svn-src-head@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-head@freebsd.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: SVN commit messages for the src tree for head/-current
 <svn-src-head.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-src-head>,
 <mailto:svn-src-head-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-head/>;
List-Post: <mailto:svn-src-head@freebsd.org>
List-Help: <mailto:svn-src-head-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-src-head>,
 <mailto:svn-src-head-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>