Bug #5271
closed
Happens on many other protocols than DNS, DNS is just an example
The main issue here appears to be the use of a vector for transactions. Add new transactions is relatively cheap due to how vectors grow, but removing from anywhere other than the end results in all the memory being moved up, this is particularly bad when removing from the front. Take DNS, if you can get thousands of transactions added with one segment of input, due to Suricata transaction handling, they will be removed in order from the front. So thats multiple removals and multiple memmmoves.
The fix for parsers where transactions are normally removed in the same order as they are added, such as DNS, is a VecDeque which has efficient removals from the front. A VecDeque should also help with parsers that need to remove from the middle as its likely less memory will need to be moved due to a VecDeque internally being 2 vectors.
- Related to Bug #5278: app-layer: Allow for non slice based transaction containers in generate get iterator (rust) added
- Assignee changed from Philippe Antoine to Jason Ish
Jason, you seem to have already begun to work on this
List of protocols found by oss-fuzz :
mqtt, http2, dcerpc, rdp, pgsql...
Philippe Antoine wrote in #note-4:
Jason, you seem to have already begun to work on this
List of protocols found by oss-fuzz :
mqtt, http2, dcerpc, rdp, pgsql...
Started here: https://github.com/OISF/suricata/pull/7313
Its starts with a change to the trait to allow for VecDeque as a transaction store. Should be trivial to convert affected protocols, DNS was only a few lines change with the update to the trait.
List of protocols found by oss-fuzz :
mqtt, http2, dcerpc, rdp, pgsql...
Any others? I'm actually thinking now that a VecDeque
is a better collection that a vec
for all parsers and should just become the default.
That is the exhaustive list as of today...
But I am ok with VecDeque being the default :-)
- Related to Bug #5314: ftp: quadratic complexity for tx iterator with linked list added
- Status changed from New to Closed
- Related to Bug #5637: quic: convert to vecdeque added
- Private changed from Yes to No
Also available in: Atom
PDF