git: f13da24715a7 - main - net/bpf: Fix writing of buffer bigger than PAGESIZE

[Kristof Provost]

On 23 Jun 2021, at 22:35, Gleb Smirnoff wrote:
>   Warner, Kristof,
>
> On Wed, Jun 23, 2021 at 04:41:01PM +0000, Warner Losh wrote:
> W>     net/bpf: Fix writing of buffer bigger than PAGESIZE
> W>
> W>     When allocating the mbuf we used m_get2 which fails
> W>     if len is superior to MJUMPAGESIZE, if its the case,
> W>     use m_getjcl instead.
> W>
> W>     Reviewed by:    kp@
> W>     PR:             205164
> W>     Pull Request:   https://github.com/freebsd/freebsd-src/pull/131
>
> m_get2() used to provide jumbo mbufs in the past, see 
> 3112ae76449ae0931d207603f14b083627bd731d.
>
> IMHO, makes sense to create m_get3() and use it in bpf. What do you 
> think?
>
Do you mean, create an m_get3() like this (untested):

	diff --git a/sys/kern/kern_mbuf.c b/sys/kern/kern_mbuf.c
	index a46c576bad90..202a1c1a5b41 100644
	--- a/sys/kern/kern_mbuf.c
	+++ b/sys/kern/kern_mbuf.c
	@@ -1370,6 +1370,51 @@ m_get2(int size, int how, short type, int flags)
	 	return (m);
	 }

	+/*
	+ * m_get3() allocates minimum mbuf that would fit "size" argument.
	+ * Unlike m_get2() is can allocate clusters up to MJUM16BYTES.
	+ */
	+struct mbuf *
	+m_get3(int size, int how, short type, int flags)
	+{
	+	struct mb_args args;
	+	struct mbuf *m, *n;
	+	uma_zone_t zone;
	+
	+	args.flags = flags;
	+	args.type = type;
	+
	+	if (size <= MHLEN || (size <= MLEN && (flags & M_PKTHDR) == 0))
	+		return (uma_zalloc_arg(zone_mbuf, &args, how));
	+	if (size <= MCLBYTES)
	+		return (uma_zalloc_arg(zone_pack, &args, how));
	+
	+	if (size > MJUM16BYTES)
	+		return (NULL);
	+
	+	m = uma_zalloc_arg(zone_mbuf, &args, how);
	+	if (m == NULL)
	+		return (NULL);
	+
	+#if MJUMPAGESIZE != MCLBYTES
	+	if (size <= MJUMPAGESIZE)
	+		zone = zone_jumbop;
	+	else
	+#endif
	+	if (size <= MJUM9BYTES)
	+		zone = zone_jumbo9;
	+	else
	+		zone = zone_jumbo16;
	+
	+	n = uma_zalloc_arg(zone_jumbop, m, how);
	+	if (n == NULL) {
	+		uma_zfree(zone_mbuf, m);
	+		return (NULL);
	+	}
	+
	+	return (m);
	+}
	+
	 /*
	  * m_getjcl() returns an mbuf with a cluster of the specified size 
attached.
	  * For size it takes MCLBYTES, MJUMPAGESIZE, MJUM9BYTES, MJUM16BYTES.
	diff --git a/sys/net/bpf.c b/sys/net/bpf.c
	index ec05dd6d337b..ef7285241fdf 100644
	--- a/sys/net/bpf.c
	+++ b/sys/net/bpf.c
	@@ -641,15 +641,7 @@ bpf_movein(struct uio *uio, int linktype, struct 
ifnet *ifp, struct mbuf **mp,
	 	if (len < hlen || len - hlen > ifp->if_mtu)
	 		return (EMSGSIZE);

	-	/* Allocate a mbuf for our write, since m_get2 fails if len >= to 
MJUMPAGESIZE, use m_getjcl for bigger buffers */
	-	if (len < MJUMPAGESIZE)
	-		m = m_get2(len, M_WAITOK, MT_DATA, M_PKTHDR);
	-	else if (len <= MJUM9BYTES)
	-		m = m_getjcl(M_WAITOK, MT_DATA, M_PKTHDR, MJUM9BYTES);
	-	else if (len <= MJUM16BYTES)
	-		m = m_getjcl(M_WAITOK, MT_DATA, M_PKTHDR, MJUM16BYTES);
	-	else
	-		m = NULL;
	+	m = m_get3(len, M_WAITOK, MT_DATA, M_PKTHDR);
	 	if (m == NULL)
	 		return (EIO);
	 	m->m_pkthdr.len = m->m_len = len;
	diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
	index e37b872c74fe..b9211aaedc3c 100644
	--- a/sys/sys/mbuf.h
	+++ b/sys/sys/mbuf.h
	@@ -828,6 +828,7 @@ u_int		 m_fixhdr(struct mbuf *);
	 struct mbuf	*m_fragment(struct mbuf *, int, int);
	 void		 m_freem(struct mbuf *);
	 struct mbuf	*m_get2(int, int, short, int);
	+struct mbuf	*m_get3(int, int, short, int);
	 struct mbuf	*m_getjcl(int, short, int, int);
	 struct mbuf	*m_getm2(struct mbuf *, int, int, short, int);
	 struct mbuf	*m_getptr(struct mbuf *, int, int *);


—
Kristof