https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=212749
Bug ID: 212749
Summary: bridge fragment can leak mbufs
Product: Base System
Version: CURRENT
Hardware: Any
OS: Any
Status: New
Severity: Affects Only Me
Priority: ---
Component: kern
Assignee: freebsd-bugs@FreeBSD.org
Reporter: fodillemlinka...@gmail.com
Created attachment 174858
--> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=174858&action=edit
patch to the bridge code that solves the problem
Hello,
It has come to my attention that when bridge_fragment() has to fragment an IP
packet it can cause mbuf leaks if, for example m_pullup() ends up allocating a
new buffer at the front of the mbuf chain or if the calls to M_PREPEND would
fail due to the mbuf pool being depleted.
One can easily convince himself that something is wrong by observing that, in
bridge_pfil(), bridge_fragment() gets passed an mbuf pointer and if the call to
m_pullup() in bridge_fragment ends up allocating a new mbuf at the front of the
chain, then whatever happens next isn't going to give bridge_pfil() the updated
mbuf pointer. You can see below this is why I had to change the arguments to
bridge_fragment to take a double pointer to the mbuf.
The problem actually goes a lot deeper since, after having called
ip_fragment(), bridge_fragment() simply goes over the list of packets it got
and carelessly calls into M_PREPEND() to make space for the ethernet header.
Now this isn't a problem if M_PREPEND() can find the space at the beginning of
the mbuf but will wreak havoc if it must take the least used code path and
prepend an mbuf to the chain. In this case we have m0 pointing to a newly
allocated mbuf but the list (walked through m_nextpkt) will never get updated
to point to m0 (the previous mbuf's m_nextpkt pointer should now point to the
new m0).
And it goes on, the error case isn't' handled properly since if MPREPEND
deleted m0 we not have a list of packets that is potentially pointed to an
already freed mbuf. Nowhere can we see that m_nextpkt is updated to take care
of that case. This is why in my patch below I detach each m0 from the packet
list so if a failure occurs in M_PREPEND of if a new mbuf is added at the
beginning of the chain I can keep the list of packets updated.
Same thing with the goto out; at the end. If we lost a fragment while adding
ethernet header to it we must free the entire chain of packets or we will leak
mbufs. There is no point in sending fragments if we aren't going to send them
all.
To convince yourself of this problem, one can simple create, on his favorite
bridge one 'reass' rule in ipfw. I use the following rule:
00080 253 27878 reass ip from any to any { proto udp or proto gre }
Then send (assuming your MTU is 1500), a big udp packet using iperf. I use this
command:
iperf -c 10.10.73.2 -u -b16Mb -t 240 -i 10 -l 5000
After iperf has finished, look at your mbuf count with netstat -m.
I have made a patch to fix this problem (attached). The patch was tested under
load when we actually do run out of mbuf to trigger the error cases as well as
with all sorts of number of fragments (varying the initial packet size).
I see this seems to affect all modern versions of FreeBSD, please feel free to
test or contest. I would like to see this added to FreeBSD eventually in one
shape or another.
Best regards,
Karim.
--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
freebsd-bugs@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
