Fix some long standing bugs in writing to the BPF device attached to

a DLT_NULL interface. In particular:

        1) Consistently use type u_int32_t for the header of a
           DLT_NULL device - it continues to represent the address
           family as always.
        2) In the DLT_NULL case get bpf_movein to store the u_int32_t
           in a sockaddr rather than in the mbuf, to be consistent
           with all the DLT types.
        3) Consequently fix a bug in bpf_movein/bpfwrite which
           only permitted packets up to 4 bytes less than the MTU
           to be written.
        4) Fix all DLT_NULL devices to have the code required to
           allow writing to their bpf devices.
        5) Move the code to allow writing to if_lo from if_simloop
           to looutput, because it only applies to DLT_NULL devices
           but was being applied to other devices that use if_simloop
           possibly incorrectly.

PR:		82157
Submitted by:	Matthew Luckie <mjl@luckie.org.nz>
Approved by:	re (scottl)

8 files changed, 75 insertions, 48 deletions

diff --git a/sys/net/bpf.c b/sys/net/bpf.c
index ac81ff1..514d6d2 100644
--- a/sys/net/bpf.c
+++ b/sys/net/bpf.c
@@ -106,8 +106,8 @@ static void	bpf_attachd(struct bpf_d *d, struct bpf_if *bp);
 static void	bpf_detachd(struct bpf_d *d);
 static void	bpf_freed(struct bpf_d *);
 static void	bpf_mcopy(const void *, void *, size_t);
-static int	bpf_movein(struct uio *, int,
-		    struct mbuf **, struct sockaddr *, int *);
+static int	bpf_movein(struct uio *, int, int,
+		    struct mbuf **, struct sockaddr *);
 static int	bpf_setif(struct bpf_d *, struct ifreq *);
 static void	bpf_timed_out(void *);
 static __inline void
@@ -148,9 +148,10 @@ static struct filterops bpfread_filtops =
 	{ 1, NULL, filt_bpfdetach, filt_bpfread };
 
 static int
-bpf_movein(uio, linktype, mp, sockp, datlen)
+bpf_movein(uio, linktype, mtu, mp, sockp)
 	struct uio *uio;
-	int linktype, *datlen;
+	int linktype;
+	int mtu;
 	struct mbuf **mp;
 	struct sockaddr *sockp;
 {
@@ -187,11 +188,19 @@ bpf_movein(uio, linktype, mp, sockp, datlen)
 		break;
 
 	case DLT_RAW:
-	case DLT_NULL:
 		sockp->sa_family = AF_UNSPEC;
 		hlen = 0;
 		break;
 
+	case DLT_NULL:
+		/*
+		 * null interface types require a 4 byte pseudo header which
+		 * corresponds to the address family of the packet.
+		 */
+		sockp->sa_family = AF_UNSPEC;
+		hlen = 4;
+		break;
+
 	case DLT_ATM_RFC1483:
 		/*
 		 * en atm driver requires 4-byte atm pseudo header.
@@ -212,7 +221,10 @@ bpf_movein(uio, linktype, mp, sockp, datlen)
 	}
 
 	len = uio->uio_resid;
-	*datlen = len - hlen;
+
+	if (len - hlen > mtu)
+		return (EMSGSIZE);
+
 	if ((unsigned)len > MCLBYTES)
 		return (EIO);
 
@@ -569,7 +581,6 @@ bpfwrite(dev, uio, ioflag)
 	struct mbuf *m;
 	int error;
 	struct sockaddr dst;
-	int datlen;
 
 	if (d->bd_bif == NULL)
 		return (ENXIO);
@@ -583,15 +594,10 @@ bpfwrite(dev, uio, ioflag)
 		return (0);
 
 	bzero(&dst, sizeof(dst));
-	error = bpf_movein(uio, (int)d->bd_bif->bif_dlt, &m, &dst, &datlen);
+	error = bpf_movein(uio, (int)d->bd_bif->bif_dlt, ifp->if_mtu, &m, &dst);
 	if (error)
 		return (error);
 
-	if (datlen > ifp->if_mtu) {
-		m_freem(m);
-		return (EMSGSIZE);
-	}
-
 	if (d->bd_hdrcmplt)
 		dst.sa_family = pseudo_AF_HDRCMPLT;
 
diff --git a/sys/net/if_disc.c b/sys/net/if_disc.c
index 7789b29..d94eeeb 100644
--- a/sys/net/if_disc.c
+++ b/sys/net/if_disc.c
@@ -102,7 +102,7 @@ disc_clone_create(struct if_clone *ifc, int unit)
 	ifp->if_addrlen = 0;
 	ifp->if_snd.ifq_maxlen = 20;
 	if_attach(ifp);
-	bpfattach(ifp, DLT_NULL, sizeof(u_int));
+	bpfattach(ifp, DLT_NULL, sizeof(u_int32_t));
 	mtx_lock(&disc_mtx);
 	LIST_INSERT_HEAD(&disc_softc_list, sc, sc_list);
 	mtx_unlock(&disc_mtx);
@@ -176,15 +176,14 @@ static int
 discoutput(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
     struct rtentry *rt)
 {
+	u_int32_t af;
 
 	M_ASSERTPKTHDR(m);
 
-	/* BPF write needs to be handled specially */
+	/* BPF writes need to be handled specially. */
 	if (dst->sa_family == AF_UNSPEC) {
-		dst->sa_family = *(mtod(m, int *));
-		m->m_len -= sizeof(int);
-		m->m_pkthdr.len -= sizeof(int);
-		m->m_data += sizeof(int);
+		bcopy(dst->sa_data, &af, sizeof(af));
+		dst->sa_family = af;
 	}
 
 	if (ifp->if_bpf) {
diff --git a/sys/net/if_faith.c b/sys/net/if_faith.c
index 5835811..36bc0cd 100644
--- a/sys/net/if_faith.c
+++ b/sys/net/if_faith.c
@@ -187,7 +187,7 @@ faith_clone_create(ifc, unit)
 	ifp->if_addrlen = 0;
 	ifp->if_snd.ifq_maxlen = ifqmaxlen;
 	if_attach(ifp);
-	bpfattach(ifp, DLT_NULL, sizeof(u_int));
+	bpfattach(ifp, DLT_NULL, sizeof(u_int32_t));
 	mtx_lock(&faith_mtx);
 	LIST_INSERT_HEAD(&faith_softc_list, sc, sc_list);
 	mtx_unlock(&faith_mtx);
@@ -225,19 +225,18 @@ faithoutput(ifp, m, dst, rt)
 	struct rtentry *rt;
 {
 	int isr;
+	u_int32_t af;
 
 	M_ASSERTPKTHDR(m);
 
-	/* BPF write needs to be handled specially */
+	/* BPF writes need to be handled specially. */
 	if (dst->sa_family == AF_UNSPEC) {
-		dst->sa_family = *(mtod(m, int *));
-		m->m_len -= sizeof(int);
-		m->m_pkthdr.len -= sizeof(int);
-		m->m_data += sizeof(int);
+		bcopy(dst->sa_data, &af, sizeof(af));
+		dst->sa_family = af;
 	}
 
 	if (ifp->if_bpf) {
-		u_int32_t af = dst->sa_family;
+		af = dst->sa_family;
 		bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m);
 	}
 
diff --git a/sys/net/if_gif.c b/sys/net/if_gif.c
index 4c243b6..3291321 100644
--- a/sys/net/if_gif.c
+++ b/sys/net/if_gif.c
@@ -179,7 +179,7 @@ gifattach0(sc)
 	GIF2IFP(sc)->if_output = gif_output;
 	GIF2IFP(sc)->if_snd.ifq_maxlen = IFQ_MAXLEN;
 	if_attach(GIF2IFP(sc));
-	bpfattach(GIF2IFP(sc), DLT_NULL, sizeof(u_int));
+	bpfattach(GIF2IFP(sc), DLT_NULL, sizeof(u_int32_t));
 	if (ng_gif_attach_p != NULL)
 		(*ng_gif_attach_p)(GIF2IFP(sc));
 }
@@ -348,6 +348,7 @@ gif_output(ifp, m, dst, rt)
 	struct m_tag *mtag;
 	int error = 0;
 	int gif_called;
+	u_int32_t af;
 
 #ifdef MAC
 	error = mac_check_ifnet_transmit(ifp, m);
@@ -404,8 +405,14 @@ gif_output(ifp, m, dst, rt)
 		goto end;
 	}
 
+	/* BPF writes need to be handled specially. */
+	if (dst->sa_family == AF_UNSPEC) {
+		bcopy(dst->sa_data, &af, sizeof(af));
+		dst->sa_family = af;
+	}
+
 	if (ifp->if_bpf) {
-		u_int32_t af = dst->sa_family;
+		af = dst->sa_family;
 		bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m);
 	}
 	ifp->if_opackets++;	
diff --git a/sys/net/if_gre.c b/sys/net/if_gre.c
index 407784d..eae1f16 100644
--- a/sys/net/if_gre.c
+++ b/sys/net/if_gre.c
@@ -233,6 +233,7 @@ gre_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
 	struct ip *ip;
 	u_int16_t etype = 0;
 	struct mobile_h mob_h;
+	u_int32_t af;
 
 	/*
 	 * gre may cause infinite recursion calls when misconfigured.
@@ -256,8 +257,14 @@ gre_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
 	gh = NULL;
 	ip = NULL;
 
+	/* BPF writes need to be handled specially. */
+	if (dst->sa_family == AF_UNSPEC) {
+		bcopy(dst->sa_data, &af, sizeof(af));
+		dst->sa_family = af;
+	}
+
 	if (ifp->if_bpf) {
-		u_int32_t af = dst->sa_family;
+		af = dst->sa_family;
 		bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m);
 	}
 
diff --git a/sys/net/if_loop.c b/sys/net/if_loop.c
index d10d859..12231b2 100644
--- a/sys/net/if_loop.c
+++ b/sys/net/if_loop.c
@@ -156,7 +156,7 @@ lo_clone_create(ifc, unit)
 	ifp->if_snd.ifq_maxlen = ifqmaxlen;
 	ifp->if_softc = sc;
 	if_attach(ifp);
-	bpfattach(ifp, DLT_NULL, sizeof(u_int));
+	bpfattach(ifp, DLT_NULL, sizeof(u_int32_t));
 	mtx_lock(&lo_mtx);
 	LIST_INSERT_HEAD(&lo_list, sc, sc_next);
 	mtx_unlock(&lo_mtx);
@@ -199,6 +199,8 @@ looutput(ifp, m, dst, rt)
 	struct sockaddr *dst;
 	register struct rtentry *rt;
 {
+	u_int32_t af;
+
 	M_ASSERTPKTHDR(m); /* check if we have the packet header */
 
 	if (rt && rt->rt_flags & (RTF_REJECT|RTF_BLACKHOLE)) {
@@ -209,6 +211,13 @@ looutput(ifp, m, dst, rt)
 
 	ifp->if_opackets++;
 	ifp->if_obytes += m->m_pkthdr.len;
+
+	/* BPF writes need to be handled specially. */
+	if (dst->sa_family == AF_UNSPEC) {
+		bcopy(dst->sa_data, &af, sizeof(af));
+		dst->sa_family = af;
+	}
+
 #if 1	/* XXX */
 	switch (dst->sa_family) {
 	case AF_INET:
@@ -249,15 +258,6 @@ if_simloop(ifp, m, af, hlen)
 	m_tag_delete_nonpersistent(m);
 	m->m_pkthdr.rcvif = ifp;
 
-	/* BPF write needs to be handled specially */
-	if (af == AF_UNSPEC) {
-		KASSERT(m->m_len >= sizeof(int), ("if_simloop: m_len"));
-		af = *(mtod(m, int *));
-		m->m_len -= sizeof(int);
-		m->m_pkthdr.len -= sizeof(int);
-		m->m_data += sizeof(int);
-	}
-
 	/* Let BPF see incoming packet */
 	if (ifp->if_bpf) {
 		if (ifp->if_bpf->bif_dlt == DLT_NULL) {
diff --git a/sys/net/if_stf.c b/sys/net/if_stf.c
index c848e2a..3ba3343 100644
--- a/sys/net/if_stf.c
+++ b/sys/net/if_stf.c
@@ -245,7 +245,7 @@ stf_clone_create(struct if_clone *ifc, char *name, size_t len)
 	ifp->if_output = stf_output;
 	ifp->if_snd.ifq_maxlen = IFQ_MAXLEN;
 	if_attach(ifp);
-	bpfattach(ifp, DLT_NULL, sizeof(u_int));
+	bpfattach(ifp, DLT_NULL, sizeof(u_int32_t));
 	mtx_lock(&stf_mtx);
 	LIST_INSERT_HEAD(&stf_softc_list, sc, sc_list);
 	mtx_unlock(&stf_mtx);
@@ -438,6 +438,7 @@ stf_output(ifp, m, dst, rt)
 	struct ip *ip;
 	struct ip6_hdr *ip6;
 	struct in6_ifaddr *ia6;
+	u_int32_t af;
 #ifdef MAC
 	int error;
 
@@ -481,6 +482,15 @@ stf_output(ifp, m, dst, rt)
 	tos = (ntohl(ip6->ip6_flow) >> 20) & 0xff;
 
 	/*
+	 * BPF writes need to be handled specially.
+	 * This is a null operation, nothing here checks dst->sa_family.
+	 */
+	if (dst->sa_family == AF_UNSPEC) {
+		bcopy(dst->sa_data, &af, sizeof(af));
+		dst->sa_family = af;
+	}
+
+	/*
 	 * Pickup the right outer dst addr from the list of candidates.
 	 * ip6_dst has priority as it may be able to give us shorter IPv4 hops.
 	 */
@@ -504,7 +514,7 @@ stf_output(ifp, m, dst, rt)
 		 * will only read from the mbuf (i.e., it won't
 		 * try to free it or keep a pointer a to it).
 		 */
-		u_int32_t af = AF_INET6;
+		af = AF_INET6;
 		bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m);
 	}
 
diff --git a/sys/net/if_tun.c b/sys/net/if_tun.c
index 310ff49..4c23fa8 100644
--- a/sys/net/if_tun.c
+++ b/sys/net/if_tun.c
@@ -285,7 +285,7 @@ tuncreate(struct cdev *dev)
 	IFQ_SET_READY(&ifp->if_snd);
 
 	if_attach(ifp);
-	bpfattach(ifp, DLT_NULL, sizeof(u_int));
+	bpfattach(ifp, DLT_NULL, sizeof(u_int32_t));
 	dev->si_drv1 = sc;
 }
 
@@ -473,6 +473,7 @@ tunoutput(
 	struct tun_softc *tp = ifp->if_softc;
 	u_short cached_tun_flags;
 	int error;
+	u_int32_t af;
 
 	TUNDEBUG (ifp, "tunoutput\n");
 
@@ -499,16 +500,14 @@ tunoutput(
 		return (EHOSTDOWN);
 	}
 
-	/* BPF write needs to be handled specially */
+	/* BPF writes need to be handled specially. */
 	if (dst->sa_family == AF_UNSPEC) {
-		dst->sa_family = *(mtod(m0, int *));
-		m0->m_len -= sizeof(int);
-		m0->m_pkthdr.len -= sizeof(int);
-		m0->m_data += sizeof(int);
+		bcopy(dst->sa_data, &af, sizeof(af));
+		dst->sa_family = af; 
 	}
 
 	if (ifp->if_bpf) {
-		uint32_t af = dst->sa_family;
+		af = dst->sa_family;
 		bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m0);
 	}