Fix bug where an mbuf was being written to without checking M_WRITABLE().

Eliminate some of the unnecessary complexity of ng_ether_glueback_header().
Simplify two functions a bit by doing the NG_FREE_META(meta) earlier.

Reviewed by:	julian, brian
MFC after:	1 week

1 files changed, 24 insertions, 57 deletions

diff --git a/sys/netgraph/ng_ether.c b/sys/netgraph/ng_ether.c
index dfb84e7..1804b87 100644
--- a/sys/netgraph/ng_ether.c
+++ b/sys/netgraph/ng_ether.c
@@ -358,15 +358,12 @@ static int
 ng_ether_glueback_header(struct mbuf **mp, struct ether_header *eh)
 {
 	struct mbuf *m = *mp;
-	uintfptr_t room;
 	int error = 0;
 
 	/*
-	 * Possibly the header is already on the front.
-	 * If this is the case so just move the markers back
-	 * to re-include it. We lucked out.
-	 * This allows us to avoid a yucky m_pullup
-	 * in later nodes if it works.
+	 * Optimize for the case where the header is already in place
+	 * at the front of the mbuf. This is actually quite likely
+	 * because many Ethernet drivers generate packets this way.
 	 */
 	if (eh == mtod(m, struct ether_header *) - 1) {
 		m->m_len += sizeof(*eh);
@@ -375,52 +372,18 @@ ng_ether_glueback_header(struct mbuf **mp, struct ether_header *eh)
 		goto done;
 	}
 
-	/*
-	 * Alternatively there may be room even though
-	 * it is stored somewhere else. If so, copy it in.
-	 * This only safe because we KNOW that this packet has
-	 * just been generated by an ethernet card, so there are
-	 * no aliases to the buffer (not so for outgoing packets).
-	 * Nearly all ethernet cards will end up producing mbufs
-	 * that fall into these cases. So we are not optimizing
-	 * contorted cases.
-	 */
-	if ((m->m_flags & M_EXT) != 0) {
-		room = mtod(m, caddr_t) - m->m_ext.ext_buf;
-		if (room > m->m_ext.ext_size)	/* garbage, fail immediately */
-			room = 0;		
-	} else
-		room = mtod(m, caddr_t) - m->m_pktdat;
-
-	/*
-	 * If we have room, just copy it and adjust
-	 */
-	if (room >= sizeof(*eh)) {
-		m->m_len += sizeof(*eh);
-		m->m_data -= sizeof(*eh);
-		m->m_pkthdr.len += sizeof(*eh);
-		goto copy;
-	}
-
-	/*
-	 * Doing anything more is likely to get more
-	 * expensive than it's worth..
-	 * it's probable that everything else is in one
-	 * big lump. The next node will do an m_pullup()
-	 * for exactly the amount of data it needs and
-	 * hopefully everything after that will not
-	 * need one. So let's just use M_PREPEND.
-	 */
-	M_PREPEND(m, sizeof (*eh), M_DONTWAIT);
+	/* Prepend the header back onto the front of the mbuf */
+	M_PREPEND(m, sizeof(*eh), M_DONTWAIT);
 	if (m == NULL) {
 		error = ENOBUFS;
 		goto done;
 	}
 
-copy:
-	/* Copy header and return (possibly new) mbuf */
-	bcopy((caddr_t)eh, mtod(m, struct ether_header *), sizeof(*eh));
+	/* Copy header into front of mbuf */
+	bcopy(eh, mtod(m, void *), sizeof(*eh));
+
 done:
+	/* Done */
 	*mp = m;
 	return error;
 }
@@ -629,34 +592,39 @@ ng_ether_rcv_lower(node_p node, struct mbuf *m, meta_p meta)
 	const priv_p priv = NG_NODE_PRIVATE(node);
  	struct ifnet *const ifp = priv->ifp;
 
+	/* Discard meta info */
+	NG_FREE_META(meta);
+
 	/* Check whether interface is ready for packets */
 	if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING)) {
 		NG_FREE_M(m);
-		NG_FREE_META(meta);
 		return (ENETDOWN);
 	}
 
 	/* Make sure header is fully pulled up */
 	if (m->m_pkthdr.len < sizeof(struct ether_header)) {
 		NG_FREE_M(m);
-		NG_FREE_META(meta);
 		return (EINVAL);
 	}
 	if (m->m_len < sizeof(struct ether_header)
-	    && (m = m_pullup(m, sizeof(struct ether_header))) == NULL) {
-		NG_FREE_META(meta);
+	    && (m = m_pullup(m, sizeof(struct ether_header))) == NULL)
 		return (ENOBUFS);
-	}
 
 	/* Drop in the MAC address if desired */
 	if (priv->autoSrcAddr) {
+
+		/* Make the mbuf writable if it's not already */
+		if (!M_WRITABLE(m)
+		    && (m = m_pullup(m, sizeof(struct ether_header))) == NULL)
+			return (ENOBUFS);
+
+		/* Overwrite source MAC address */
 		bcopy((IFP2AC(ifp))->ac_enaddr,
 		    mtod(m, struct ether_header *)->ether_shost,
 		    ETHER_ADDR_LEN);
 	}
 
 	/* Send it on its way */
-	NG_FREE_META(meta);
 	return ether_output_frame(ifp, m);
 }
 
@@ -669,16 +637,16 @@ ng_ether_rcv_upper(node_p node, struct mbuf *m, meta_p meta)
 	const priv_p priv = NG_NODE_PRIVATE(node);
 	struct ether_header *eh;
 
+	/* Discard meta info */
+	NG_FREE_META(meta);
+
 	/* Check length and pull off header */
 	if (m->m_pkthdr.len < sizeof(*eh)) {
 		NG_FREE_M(m);
-		NG_FREE_META(meta);
 		return (EINVAL);
 	}
-	if (m->m_len < sizeof(*eh) && (m = m_pullup(m, sizeof(*eh))) == NULL) {
-		NG_FREE_META(meta);
+	if (m->m_len < sizeof(*eh) && (m = m_pullup(m, sizeof(*eh))) == NULL)
 		return (ENOBUFS);
-	}
 	eh = mtod(m, struct ether_header *);
 	m->m_data += sizeof(*eh);
 	m->m_len -= sizeof(*eh);
@@ -686,7 +654,6 @@ ng_ether_rcv_upper(node_p node, struct mbuf *m, meta_p meta)
 	m->m_pkthdr.rcvif = priv->ifp;
 
 	/* Route packet back in */
-	NG_FREE_META(meta);
 	ether_demux(priv->ifp, eh, m);
 	return (0);
 }