Created attachment 176852 [details]
We noticed L2TP control packets having incorrect sequence numbers causing problems talking to Cisco routers. We traced this back to /usr/src/sys/netgraph/ng_l2tp.c The code is writing to what it thinks 12 bytes of continuous memory, however this can't be guaranteed as the mbuf may have been prepended to. A call to m_pullup is needed, see patch attached.
We believe this may have manifested its self as we are sending bigger packets than the MPD software would normally send due to the addition of proxy auth AVPs, which are not in the stock distribution.
This patch was against 10.3 but will work for 11.0 and probably head as well.
Wouldn't it be enough to do that pullup in the "else" path right above it rather than outside the block? In the first case of an empty ZLB it doesn't seem to be needed?
Created attachment 177064 [details]
This version does not call m_pullup when the packet is a ZLB.
yes I think you are right, a newly allocated mbup should always have enough space for a 12 byte header. We thought maybe m_pullup was meant to be called anytime the memory was to be accessed, can't see mention of this in the man page though. I have attached an amended patch where m_pullup is only called in the case where M_PREPEND has also been used.
A commit references this bug:
Date: Thu Nov 17 14:03:44 UTC 2016
New revision: 308748
Writing out the L2TP control packet requires 12 bytes of
contiguous memory but in one path we did not always guarantee this,
thus do a m_pullup() there.
Submitted by: Joe Jones (joeknockando googlemail.com)
MFC after: 3 days