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
This commit is contained in:
luigi 2006-12-08 10:36:45 +00:00
parent d0a44b7b7c
commit c238e1bace

View File

@ -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;