Page MenuHomeFreeBSD

iflib: Fix several memory handling issues around iflib_encap()
ClosedPublic

Authored by gallatin on Fri, Aug 14, 12:52 PM.
Tags
None
Referenced Files
F166970472: D58843.id184043.diff
Tue, Aug 18, 6:47 AM
F166930430: D58843.id184067.diff
Mon, Aug 17, 8:32 PM
Unknown Object (File)
Mon, Aug 17, 11:53 AM
Unknown Object (File)
Mon, Aug 17, 4:07 AM
Unknown Object (File)
Sun, Aug 16, 1:14 PM
Unknown Object (File)
Fri, Aug 14, 7:09 PM
Unknown Object (File)
Fri, Aug 14, 7:05 PM
Unknown Object (File)
Fri, Aug 14, 12:59 PM

Details

Summary
  • Fixed memory leaks around m_dup() not freeing the original chain on failure. If we return ENOMEM, we are expected to have freed the chain, else the mbuf would be leaked. Also updated iflib_ether_pad() to follow the same structure.
  • In iflib_parse_header() o Fixed a bug where the ip/ip6 and th pointers may point into a freed chain after m_pullup. Those pointers must be reset to point into the new chain.

    o Eliminate ENXIO returns for non-TCP TSO sends (which would violate the mbuf ownership contract if they could happen). Since they cannot, I made them assertions instead.
  • in iflib_ether_pad(), return ENOMEM after freeing mbuf, so that mp_ring knows it is free. An ENOBUFS error will cause the mp_ring path to retain the mbuf and retry
  • in iflib_encap(): o Fix a leak when bus_dmamap_load_mbuf_sg() returns ENOMEM

    o Fix a use-after-free in the mp_ring path when a driver using ktls frees an mbuf and returns ENOBUFS via iflib_encap()

After this change:

mp_ring: ENOBUFS can be returned only when we run out of descriptors and the mbuf is retained

	 in that case.

simple_tx: iflib_encap() always consumes the mbuf, regardless of the return

Note that iflib_debugnet_transmit(), like simple_tx, expects that iflib_encap() always consumes
mbufs. This will be true after mp_ring is removed, and its such a rare special
case (panic dumps) that I don't think its worth fixing in the meantime.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

@sumit.saxena_broadcom.com This fixes a bug around ktls handling with mp_ring.
@imp this fixes the bnxt panic you reported to me a few weeks ago

kbowling added inline comments.
sys/net/iflib.c
3562–3563

optionally set *m_head = NULL after this to make the transition explicit

This revision is now accepted and ready to land.Fri, Aug 14, 1:19 PM
sys/net/iflib.c
3562–3563

Done, thanks, will test and push with this change if there is no more feedback