Date: Fri, 30 Jun 2017 11:42:00 -0400 From: Karim Fodil-Lemelin <kfodil-lemelin@xiplink.com> To: FreeBSD Net <freebsd-net@freebsd.org> Subject: m_move_pkthdr leaves m_nextpkt 'dangling' Message-ID: <59567148.1020902@xiplink.com>
next in thread | raw e-mail | index | archive | help
Hi,
As many of you know, when dealing with IP fragments the kernel will
build a list of packets (fragments) chained together through the
m_nextpkt pointer. This is all good until someone tries to do a
M_PREPEND on one of the packet in the chain and the M_PREPEND has to
create an extra mbuf to prepend at the beginning of the chain.
When doing so m_move_pkthdr is called to copy the current PKTHDR fields
(tags and flags) to the mbuf that was prepended. The function also does:
to->m_pkthdr = from->m_pkthdr;
This, for the case I am interested in, essentially leaves the 'from'
mbuf with a dangling pointer m_nextpkt pointing to the next fragment.
While this is mostly harmless because only mbufs of pkthdr types are
supposed to have m_nextpkt it triggers some panics when running with
INVARIANTS in NetGraph (see ng_base.c :: CHECK_DATA_MBUF(m)):
...
if (n->m_nextpkt != NULL) \
panic("%s: m_nextpkt", __func__); \
}
...
So I would like to propose the following patch:
@@ -442,10 +442,11 @@ m_move_pkthdr(struct mbuf *to, struct mbuf *from)
if ((to->m_flags & M_EXT) == 0)
to->m_data = to->m_pktdat;
to->m_pkthdr = from->m_pkthdr; /* especially tags */
SLIST_INIT(&from->m_pkthdr.tags); /* purge tags from src */
from->m_flags &= ~M_PKTHDR;
+ from->m_nextpkt = NULL;
}
It will reset the m_nextpkt so we don't have two mbufs pointing to the
same next packet. This is fairly harmless and solves a problem for us
here at XipLink.
Best regards,
Karim.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?59567148.1020902>