Bug 220882 - m_move_pkthdr leaves m_nextpkt 'dangling'
Summary: m_move_pkthdr leaves m_nextpkt 'dangling'
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-net mailing list
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-07-20 15:16 UTC by fodillemlinkarim
Modified: 2017-07-24 14:48 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description fodillemlinkarim 2017-07-20 15:16:11 UTC
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.