Security #6796
closedoutput/filestore: slowdown because of running OutputTxLog on useless packets
Description
Found by oss-fuzz
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66918
Reproducer is with attached pcap and force-filestore: yes or similar
This pcap can be seen as a simple HTTP request, and then we get from the server many one-byte packets that are not yet acknowledged by the client.
This is a bit similar to #6299 - do not run detection if nothing has been updated, but here it is tx logging and not detection
FLOW_TS_APP_UPDATED gets set with UPDATE_DIR_OPPOSING when the server acknowledges the request
And it does not get reset until PKT_IS_TOSERVER(p)
OutputTxLog. now only checks if flow flags have app update, but not if it is a fresh one
Files
Updated by Philippe Antoine 2 months ago
- Status changed from New to In Review
- Label Needs backport to 6.0, Needs backport to 7.0 added
Gitlab MR
Updated by Philippe Antoine 2 months ago
- Label deleted (
Needs backport to 6.0, Needs backport to 7.0)
Updated by Victor Julien 2 months ago
Can we get into this exact same situation if instead of useless packets we get tiny packets that xfer a body for example? So each packet does in fact update the state?
Updated by Philippe Antoine 2 months ago
Victor Julien wrote in #note-7:
Can we get into this exact same situation if instead of useless packets we get tiny packets that xfer a body for example? So each packet does in fact update the state?
I wrote "and then we get from the server many one-byte packets that are not yet acknowledged by the client."
So, these are tiny packets that transfer a body.
They are useless just until the client acks them which happens at the end of the pcap.
Do you mean having the same pcap with stream.inline mode ? Or that each tiny packet with body gets acked by the client ?
Or what do I not understand in this question ?
Updated by Victor Julien 2 months ago
What if they are acknowledged? Or indeed if we are in inline mode and have to inspect each unack'd packet? I think my concern is that we should not timeout if we do this type of inspection/logging per packet, for any packet size or type.
Updated by Victor Julien 2 months ago
- Subject changed from output/filestore: timeout because of running OutputTxLog on useless packets to output/filestore: slowdown because of running OutputTxLog on useless packets
Updated by Victor Julien 2 months ago
Unneeded snprintf call in filestore should get optimized.
Updated by Philippe Antoine 2 months ago
Victor Julien wrote in #note-9:
What if they are acknowledged? Or indeed if we are in inline mode and have to inspect each unack'd packet? I think my concern is that we should not timeout if we do this type of inspection/logging per packet, for any packet size or type.
You look right : with my patch, I go from 2 seconds to 30 seconds by setting stream.inline=true
61% is spent in OutputTxLogFiles. and 33% in snprintf in OutputFilestoreLogger
Updated by Philippe Antoine 2 months ago
Updated by Philippe Antoine 2 months ago
- Status changed from In Review to Resolved
Updated by Philippe Antoine about 2 months ago
- Status changed from Resolved to Closed