svn commit: r357233 - head/sys/net

[Kristof Provost]

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 at 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