[patch] bridge_fragment can leak mbufs

[Karim Fodil-Lemelin]

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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: bridge_fragment.patch
Type: text/x-patch
Size: 3647 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-net/attachments/20160916/ade1c7c4/attachment.bin>