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 10 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 10 months ago
- Label deleted (
Needs backport to 6.0, Needs backport to 7.0)
Updated by Victor Julien 10 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 10 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 10 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 10 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 10 months ago
Unneeded snprintf call in filestore should get optimized.
Updated by Philippe Antoine 10 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 10 months ago
Updated by Philippe Antoine 10 months ago
- Status changed from In Review to Resolved
Updated by Philippe Antoine 10 months ago
- Status changed from Resolved to Closed