From 294dd290c606d31918e2208aa993e009ec0e6cfa Mon Sep 17 00:00:00 2001 From: Luigi Rizzo Date: Fri, 8 Dec 2006 10:36:45 +0000 Subject: Fix an oscure bug triggered by a recent change in kern_socket.c. The symptoms were that outgoing DHCP requests for diskless kernels had the IP header corrupt. After long investigations, the source of the problem was found in ether_output() - for SIMPLEX interfaces and broadcast traffic, a copy of the packet is passed back to the kernel through if_simloop(). However if_simloop() modifies the mbuf, while the copy obtained through m_copym() is a readonly one. The bug has been there forever, but it has been triggered only recently by a change in sosend_dgram() which passed down mbufs with sufficient space to prepend the header. This fix is trivial - use m_dup() instead of m_copy() to create the copy. As an alternative, we could try and modify if_simloop() to play safely with readonly mbufs, but i don't think it is worthwhile because 1) this is a relatively infrequent code path so we do not need to worry too much about performance, and 2) the cost of doing an extra m_pullup in if_simloop() is probably the same as doing the copy of the cluster, anyways. MFC after: 1 week --- sys/net/if_ethersubr.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c index 5a507e3366dd..6893acdb6d00 100644 --- a/sys/net/if_ethersubr.c +++ b/sys/net/if_ethersubr.c @@ -306,7 +306,15 @@ ether_output(struct ifnet *ifp, struct mbuf *m, if (m->m_flags & M_BCAST) { struct mbuf *n; - if ((n = m_copy(m, 0, (int)M_COPYALL)) != NULL) { + /* + * Because if_simloop() modifies the packet, we need a + * writable copy through m_dup() instead of a readonly + * one as m_copy[m] would give us. The alternative would + * be to modify if_simloop() to handle the readonly mbuf, + * but performancewise it is mostly equivalent (trading + * extra data copying vs. extra locking). + */ + if ((n = m_dup(m, M_DONTWAIT)) != NULL) { n->m_pkthdr.csum_flags |= csum_flags; if (csum_flags & CSUM_DATA_VALID) n->m_pkthdr.csum_data = 0xffff; -- cgit v1.3