On 12/27/2017 12:59, Andrey V. Elsukov wrote:
On 27.12.2017 23:09, Navdeep Parhar wrote:
It is not clear to me why it helps. The panic happens on outbound path,
where mbuf should be allocated by network stack and should be writeable.
ip_reass() usually used on inbound path. I think the patch just hides
the problem in another place.
Do you mean that cxgbe can produce !WRITEABLE mbuf for received packet
and then pass it to the network stack?
Yes, cxgbe does that. But I think the real bug here is in ip_reass
because it doesn't properly get rid of the pkthdr of the fragments while
creating the reassembled datagram. cxgbe happens to trip on this easily
because it often creates !WRITEABLE mbufs.
From the quick look, I don't see the code in netipsec and in crypto,
that does check mbuf is WRITEABLE. It is expected that in most cases for
received mbuf the data will be decrypted and copied back into the given
buffer. Can this lead to memory corruption?
This should fix it:
https://people.freebsd.org/~np/ip_reass_demotehdr.diff
It will also fix leaks in configurations where mbuf tags are in use by
default (for example with MAC), ip_reass is involved during rx, and the
mbuf chain never gets m_demote'd elsewhere (meaning ip_reass should have
freed the tags itself).
I think such chain with several mbufs with M_PKTHDR flag is created with
m_cat() due to !WRITEABLE mbufs. And when mbuf chain will be freed, the
tags chain will be also destroyed by mbuf zone destructor.
I see m_freem/m_free will do the right thing but such a chain isn't
legal. m_unshare is complaining about it here. m_sanity on the chain
will fail too.
m_cat says it will leave the pkthdr alone so it is working as
advertised. It's the caller's job to clean up headers etc. to keep the
mbuf chain valid.
If you think it solves the problem, the IPv6 fragment reassembly
probably needs the same code. But I think that M_WRITEABLE flag is not
properly handled is the problem too.
I think M_WRITEABLE is being handled properly here. m_unshare deals
with the chain just fine apart from this assert about multiple M_PKTHDR.
I'll fix IP6 reassembly too and post to phabricator if the change looks ok?
Regards,
Navdeep
_______________________________________________
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
