Merge remote-tracking branch 'origin/topic/timw/analyzer-crash'

* origin/topic/timw/analyzer-crash:
  Fix crash in Analyzer::ForwardPacket due to recursive analyzer calls.
This commit is contained in:
Jon Siwek 2021-03-26 16:39:22 -07:00
commit a0859276bf
7 changed files with 36 additions and 7 deletions

13
CHANGES
View file

@ -1,3 +1,16 @@
4.1.0-dev.421 | 2021-03-26 16:39:22 -0700
* Fix crash in Analyzer::ForwardPacket due to recursive analyzer calls. (Tim Wojtulewicz, Corelight)
The change in 44f558df7b5a85bae40945de653bcb2448e0a7f4 that made analyzer_list
a std::vector instead of a std::list doesn't take into account that in some
cases an analyzer may chain back into itself, such as with UDP-in-UDP tunnels.
In these cases, the second call to ForwardPacket may cause iterator
invalidation, leading to a crash, so this reverts back to using an std::list.
* Include git sha in request to benchmark host (Tim Wojtulewicz, Corelight)
4.1.0-dev.417 | 2021-03-25 11:37:55 -0700 4.1.0-dev.417 | 2021-03-25 11:37:55 -0700
* test suite update due to factoring out coerce_to_record() (Vern Paxson, Corelight) * test suite update due to factoring out coerce_to_record() (Vern Paxson, Corelight)

5
NEWS
View file

@ -49,11 +49,6 @@ New Functionality
Changed Functionality Changed Functionality
--------------------- ---------------------
- The ``zeek::analyzer::analyzer_list`` type-alias changed from an
``std::list`` to ``std::vector`` which, in practice, is not expected to be
used from plugins in API-incompatible way and may result in ~1-2% overall
performance benefit.
Removed Functionality Removed Functionality
--------------------- ---------------------

View file

@ -1 +1 @@
4.1.0-dev.417 4.1.0-dev.421

View file

@ -38,7 +38,12 @@ class AnalyzerTimer;
class SupportAnalyzer; class SupportAnalyzer;
class OutputHandler; class OutputHandler;
using analyzer_list = std::vector<Analyzer*>; // This needs to remain a std::list because of the usage of iterators in the
// Analyzer::Forward methods. These methods have the chance to loop back
// into the same analyzer in the case of tunnels. If the recursive call adds
// to the children list, it can invalidate iterators in the outer call,
// causing a crash.
using analyzer_list = std::list<Analyzer*>;
typedef uint32_t ID; typedef uint32_t ID;
typedef void (Analyzer::*analyzer_timer_func)(double t); typedef void (Analyzer::*analyzer_timer_func)(double t);

View file

@ -0,0 +1,12 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
#separator \x09
#set_separator ,
#empty_field (empty)
#unset_field -
#path tunnel
#open XXXX-XX-XX-XX-XX-XX
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p tunnel_type action
#types time string addr port addr port enum enum
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 ::38:5f55:6265:726b 25977 2090:9090:9090:9090:9090:9000:: 25964 Tunnel::TEREDO Tunnel::DISCOVER
XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 ::38:5f55:6265:726b 25977 2090:9090:9090:9090:9090:9000:: 25964 Tunnel::TEREDO Tunnel::CLOSE
#close XXXX-XX-XX-XX-XX-XX

Binary file not shown.

View file

@ -0,0 +1,4 @@
# @TEST-EXEC: zeek -r $TRACES/tunnels/teredo-udp-in-udp.pcap %INPUT
# @TEST-EXEC: btest-diff tunnel.log
@load base/frameworks/tunnels