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 <[email protected]>
...
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
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
