From 6b1e796df72f3ba2bb1e209c28e672500142b823 Mon Sep 17 00:00:00 2001 From: Dominik Charousset Date: Thu, 12 May 2022 06:54:52 +0200 Subject: [PATCH 1/2] Fix UB during early shutdown on OpenSSL state --- src/zeek-setup.cc | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index 226e5c8188..e9255c4f01 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -823,8 +823,21 @@ SetupResult setup(int argc, char** argv, Options* zopts) if ( supervisor_mgr ) supervisor_mgr->InitPostScript(); + // After spinning up Broker, we have background threads running now. If + // we exit early, we need to shut down at least Broker to get a clean + // program exit. Otherwise, we may run into undefined behavior, e.g., if + // Broker is still accessing OpenSSL but OpenSSL has already cleaned up + // its state due to calling exit(). + auto early_shutdown = [] + { + broker_mgr->Terminate(); + delete iosource_mgr; + delete telemetry_mgr; + }; + if ( options.print_plugins ) { + early_shutdown(); bool success = show_plugins(options.print_plugins); exit(success ? 0 : 1); } @@ -843,7 +856,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) if ( reporter->Errors() > 0 ) { - delete dns_mgr; + early_shutdown(); exit(1); } @@ -880,7 +893,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) rule_matcher = new RuleMatcher(options.signature_re_level); if ( ! rule_matcher->ReadFiles(all_signature_files) ) { - delete dns_mgr; + early_shutdown(); exit(1); } @@ -913,6 +926,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) if ( analysis_options.usage_issues > 0 ) analyze_scripts(); + early_shutdown(); exit(reporter->Errors() != 0); } @@ -921,8 +935,11 @@ SetupResult setup(int argc, char** argv, Options* zopts) analyze_scripts(); if ( analysis_options.report_recursive ) + { // This option is report-and-exit. + early_shutdown(); exit(0); + } if ( dns_type != DNS_PRIME ) run_state::detail::init_run(options.interface, options.pcap_file, @@ -951,7 +968,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) reporter->FatalError("can't update DNS cache"); event_mgr.Drain(); - delete dns_mgr; + early_shutdown(); exit(0); } @@ -968,6 +985,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) id->DescribeExtended(&desc); fprintf(stdout, "%s\n", desc.Description()); + early_shutdown(); exit(0); } From 327f5e76f99d968676a994fcb00b7fc0375adf64 Mon Sep 17 00:00:00 2001 From: Dominik Charousset Date: Thu, 12 May 2022 07:17:41 +0200 Subject: [PATCH 2/2] Fix formatting --- src/zeek-setup.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index e9255c4f01..f682d39118 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -829,11 +829,11 @@ SetupResult setup(int argc, char** argv, Options* zopts) // Broker is still accessing OpenSSL but OpenSSL has already cleaned up // its state due to calling exit(). auto early_shutdown = [] - { + { broker_mgr->Terminate(); delete iosource_mgr; delete telemetry_mgr; - }; + }; if ( options.print_plugins ) {