From 4aa68f9961b617063bc8e865b5b307bc2cd90461 Mon Sep 17 00:00:00 2001 From: levy Date: Wed, 3 Mar 2021 19:03:18 +0800 Subject: [PATCH 1/8] fix: fix heap-use-after-free in perf_counters --- src/runtime/service_api_c.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/runtime/service_api_c.cpp b/src/runtime/service_api_c.cpp index 60ffc7aaca..09a53d02c9 100644 --- a/src/runtime/service_api_c.cpp +++ b/src/runtime/service_api_c.cpp @@ -46,6 +46,8 @@ #ifdef DSN_ENABLE_GPERF #include +#include + #endif #include "service_engine.h" @@ -292,6 +294,10 @@ extern void dsn_core_init(); inline void dsn_global_init() { + // make shared_io_service destructed after perf_counters, + // because shared_io_service will destruct the timer created by perf_counters + // It will produce heap-use-after-free error if shared_io_service destructed before + dsn::tools::shared_io_service::instance(); // make perf_counters/disk_engine destructed after service_engine, // because service_engine relies on the former to monitor // task queues length and close files. From f4e75c948be3f90fabf180c8f501b6361e0ae6b5 Mon Sep 17 00:00:00 2001 From: levy Date: Wed, 3 Mar 2021 19:07:25 +0800 Subject: [PATCH 2/8] fix --- src/runtime/service_api_c.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/runtime/service_api_c.cpp b/src/runtime/service_api_c.cpp index 09a53d02c9..a7e6cfafa8 100644 --- a/src/runtime/service_api_c.cpp +++ b/src/runtime/service_api_c.cpp @@ -296,7 +296,8 @@ inline void dsn_global_init() { // make shared_io_service destructed after perf_counters, // because shared_io_service will destruct the timer created by perf_counters - // It will produce heap-use-after-free error if shared_io_service destructed before + // It will produce heap-use-after-free error if shared_io_service destructed in front of + // perf_counters dsn::tools::shared_io_service::instance(); // make perf_counters/disk_engine destructed after service_engine, // because service_engine relies on the former to monitor From f8c733bc1f349f0d3aac869b3523ce482c83f321 Mon Sep 17 00:00:00 2001 From: levy Date: Wed, 3 Mar 2021 19:07:46 +0800 Subject: [PATCH 3/8] fix --- src/runtime/service_api_c.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/service_api_c.cpp b/src/runtime/service_api_c.cpp index a7e6cfafa8..b78dce9318 100644 --- a/src/runtime/service_api_c.cpp +++ b/src/runtime/service_api_c.cpp @@ -43,10 +43,10 @@ #include #include #include +#include #ifdef DSN_ENABLE_GPERF #include -#include #endif From ec11f8897fd0f6f0c25aaa19a92d496c9c62e79b Mon Sep 17 00:00:00 2001 From: levy Date: Wed, 3 Mar 2021 19:08:03 +0800 Subject: [PATCH 4/8] fix --- src/runtime/service_api_c.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runtime/service_api_c.cpp b/src/runtime/service_api_c.cpp index b78dce9318..05a9b5aa88 100644 --- a/src/runtime/service_api_c.cpp +++ b/src/runtime/service_api_c.cpp @@ -47,7 +47,6 @@ #ifdef DSN_ENABLE_GPERF #include - #endif #include "service_engine.h" From 95d1964a29c9eef95daf114172ff37d41585f76d Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 4 Mar 2021 10:43:08 +0800 Subject: [PATCH 5/8] fix --- src/perf_counter/perf_counters.cpp | 6 ++++++ src/runtime/service_api_c.cpp | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/perf_counter/perf_counters.cpp b/src/perf_counter/perf_counters.cpp index 732cfd4b01..9e18110d4e 100644 --- a/src/perf_counter/perf_counters.cpp +++ b/src/perf_counter/perf_counters.cpp @@ -46,6 +46,12 @@ namespace dsn { perf_counters::perf_counters() { + // make shared_io_service destructed after perf_counters, + // because shared_io_service will destruct the timer created by perf_counters + // It will produce heap-use-after-free error if shared_io_service destructed in front of + // perf_counters + dsn::tools::shared_io_service::instance(); + _perf_counters_cmd = command_manager::instance().register_command( {"perf-counters"}, "perf-counters - query perf counters, filtered by OR of POSIX basic regular expressions", diff --git a/src/runtime/service_api_c.cpp b/src/runtime/service_api_c.cpp index 05a9b5aa88..60ffc7aaca 100644 --- a/src/runtime/service_api_c.cpp +++ b/src/runtime/service_api_c.cpp @@ -43,7 +43,6 @@ #include #include #include -#include #ifdef DSN_ENABLE_GPERF #include @@ -293,11 +292,6 @@ extern void dsn_core_init(); inline void dsn_global_init() { - // make shared_io_service destructed after perf_counters, - // because shared_io_service will destruct the timer created by perf_counters - // It will produce heap-use-after-free error if shared_io_service destructed in front of - // perf_counters - dsn::tools::shared_io_service::instance(); // make perf_counters/disk_engine destructed after service_engine, // because service_engine relies on the former to monitor // task queues length and close files. From 53c2ef0bf5ee8fe1537dd9bade83295894c26cfa Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 4 Mar 2021 10:45:47 +0800 Subject: [PATCH 6/8] fix --- src/perf_counter/perf_counters.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/perf_counter/perf_counters.cpp b/src/perf_counter/perf_counters.cpp index 9e18110d4e..9d47b6542f 100644 --- a/src/perf_counter/perf_counters.cpp +++ b/src/perf_counter/perf_counters.cpp @@ -50,7 +50,7 @@ perf_counters::perf_counters() // because shared_io_service will destruct the timer created by perf_counters // It will produce heap-use-after-free error if shared_io_service destructed in front of // perf_counters - dsn::tools::shared_io_service::instance(); + tools::shared_io_service::instance(); _perf_counters_cmd = command_manager::instance().register_command( {"perf-counters"}, From fbfacb58f915a06932553236e757def1810917c8 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 4 Mar 2021 13:22:01 +0800 Subject: [PATCH 7/8] fix --- src/runtime/service_api_c.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/runtime/service_api_c.cpp b/src/runtime/service_api_c.cpp index 60ffc7aaca..09ebb63e4f 100644 --- a/src/runtime/service_api_c.cpp +++ b/src/runtime/service_api_c.cpp @@ -348,6 +348,18 @@ bool run(const char *config_file, bool is_server, std::string &app_list) { + // We put the loading of configuration at the beginning of this func. + // Because in dsn_global_init(), it calls perf_counters::instance(), which calls + // shared_io_service::instance(). And in the cstor of shared_io_service, it calls + // dsn_config_get_value_uint64() to load the corresponding configs. That will make + // dsn_config_get_value_uint64() get wrong value if we put dsn_config_load in behind of + // dsn_global_init() + if (!dsn_config_load(config_file, config_arguments)) { + printf("Fail to load config file %s\n", config_file); + return false; + } + dsn::flags_initialize(); + dsn_global_init(); dsn_core_init(); ::dsn::task::set_tls_dsn_context(nullptr, nullptr); @@ -358,13 +370,6 @@ bool run(const char *config_file, dsn_all.engine = &::dsn::service_engine::instance(); dsn_all.magic = 0xdeadbeef; - if (!dsn_config_load(config_file, config_arguments)) { - printf("Fail to load config file %s\n", config_file); - return false; - } - - dsn::flags_initialize(); - // pause when necessary if (dsn_config_get_value_bool("core", "pause_on_start", From 204d014de36cb4b97e162501daef70ff6f5a38c2 Mon Sep 17 00:00:00 2001 From: levy Date: Thu, 4 Mar 2021 13:41:27 +0800 Subject: [PATCH 8/8] fix --- src/runtime/service_api_c.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/service_api_c.cpp b/src/runtime/service_api_c.cpp index 09ebb63e4f..33e67ab376 100644 --- a/src/runtime/service_api_c.cpp +++ b/src/runtime/service_api_c.cpp @@ -352,7 +352,7 @@ bool run(const char *config_file, // Because in dsn_global_init(), it calls perf_counters::instance(), which calls // shared_io_service::instance(). And in the cstor of shared_io_service, it calls // dsn_config_get_value_uint64() to load the corresponding configs. That will make - // dsn_config_get_value_uint64() get wrong value if we put dsn_config_load in behind of + // dsn_config_get_value_uint64() get wrong value if we put dsn_config_load at behind of // dsn_global_init() if (!dsn_config_load(config_file, config_arguments)) { printf("Fail to load config file %s\n", config_file);