@cryptotaxi247 / netdata-1 / commits / ec48ad55e

Fix LSAN and memory leaks (#19819)

* add SIGUSR1 for exiting immediately, so that LSAN can check for leaks * fix system info leak * adapt to changed semantics of rrdhost system_info * fix memory leak in rrdcontexts when matching alerts * do not system info after it was freed * allow sentry to send its reports

Costa Tsaousis committed Mar 10, 2025 at 13:44 UTC ec48ad55ef8139ecbf1fa6c6510ad043e7a62e8c
11 files changed +46 -27
src/daemon/daemon-shutdown.c
+5 -2
@@ -318,12 +318,12 @@ void netdata_cleanup_and_exit(EXIT_REASON reason, const char *action, const char
318
319 watcher_shutdown_end();
320 watcher_thread_stop();
321 - curl_global_cleanup();
321
322 daemon_status_file_shutdown_step(NULL);
323 daemon_status_file_update_status(DAEMON_STATUS_EXITED);
324
325 #ifdef OS_WINDOWS
326 + curl_global_cleanup();
327 return;
328 #endif
329
@@ -335,12 +335,15 @@ void netdata_cleanup_and_exit(EXIT_REASON reason, const char *action, const char
335 abort();
336 } else {
337 nd_sentry_fini();
338 + curl_global_cleanup();
339 exit(ret);
340 }
341 #else
342 if(ret)
343 _exit(ret);
343 - else
344 + else {
345 + curl_global_cleanup();
346 exit(ret);
347 + }
348 #endif
349 }
src/daemon/main.c
+5 -7
@@ -1026,20 +1026,19 @@ int netdata_main(int argc, char **argv) {
1026 struct rrdhost_system_info *system_info = rrdhost_system_info_create();
1027 rrdhost_system_info_detect(system_info);
1028
1029 - // ----------------------------------------------------------------------------------------------------------------
1030 - delta_startup_time("install type");
1031 -
1029 get_install_type(system_info);
1030
1031 + set_late_analytics_variables(system_info);
1032 +
1033 // ----------------------------------------------------------------------------------------------------------------
1034 delta_startup_time("RRD structures");
1035
1036 abort_on_fatal_disable();
1038 - if(rrd_init(netdata_configured_hostname, system_info, false)) {
1039 - set_late_analytics_variables(system_info);
1037 + if (rrd_init(netdata_configured_hostname, system_info, false))
1038 fatal("Cannot initialize localhost instance with name '%s'.", netdata_configured_hostname);
1041 - }
1039 +
1040 abort_on_fatal_enable();
1041 + system_info = NULL; // system_info is now freed by rrd_init
1042
1043 // ----------------------------------------------------------------------------------------------------------------
1044 delta_startup_time("localhost labels");
@@ -1064,7 +1063,6 @@ int netdata_main(int argc, char **argv) {
1063
1064 netdata_conf_section_web();
1065
1067 - set_late_analytics_variables(system_info);
1066 for (i = 0; static_threads[i].name != NULL ; i++) {
1067 struct netdata_static_thread *st = &static_threads[i];
1068
src/daemon/signal-handler.c
+11
@@ -6,6 +6,9 @@
6 typedef enum signal_action {
7 NETDATA_SIGNAL_IGNORE,
8 NETDATA_SIGNAL_EXIT_CLEANLY,
9 +#if defined(FSANITIZE_ADDRESS)
10 + NETDATA_SIGNAL_EXIT_NOW,
11 +#endif
12 NETDATA_SIGNAL_REOPEN_LOGS,
13 NETDATA_SIGNAL_RELOAD_HEALTH,
14 NETDATA_SIGNAL_DEADLY,
@@ -23,6 +26,9 @@ static struct {
26 { SIGQUIT, "SIGQUIT", 0, NETDATA_SIGNAL_EXIT_CLEANLY, EXIT_REASON_SIGQUIT },
27 { SIGTERM, "SIGTERM", 0, NETDATA_SIGNAL_EXIT_CLEANLY, EXIT_REASON_SIGTERM },
28 { SIGHUP, "SIGHUP", 0, NETDATA_SIGNAL_REOPEN_LOGS, EXIT_REASON_NONE },
29 +#if defined(FSANITIZE_ADDRESS)
30 + { SIGUSR1, "SIGUSR1", 0, NETDATA_SIGNAL_EXIT_NOW, EXIT_REASON_NONE },
31 +#endif
32 { SIGUSR2, "SIGUSR2", 0, NETDATA_SIGNAL_RELOAD_HEALTH, EXIT_REASON_NONE },
33 { SIGBUS, "SIGBUS", 0, NETDATA_SIGNAL_DEADLY, EXIT_REASON_SIGBUS },
34 { SIGSEGV, "SIGSEGV", 0, NETDATA_SIGNAL_DEADLY, EXIT_REASON_SIGSEGV },
@@ -38,6 +44,11 @@ static void signal_handler(int signo) {
44
45 signals_waiting[i].count++;
46
47 +#if defined(FSANITIZE_ADDRESS)
48 + if(signals_waiting[i].action == NETDATA_SIGNAL_EXIT_NOW)
49 + exit(1);
50 +#endif
51 +
52 if(signals_waiting[i].action == NETDATA_SIGNAL_DEADLY) {
53 // Update the status file
54 daemon_status_file_deadly_signal_received(signals_waiting[i].reason);
src/database/contexts/api_v2_contexts_alerts.c
+4 -3
@@ -123,9 +123,10 @@ bool rrdcontext_matches_alert(struct rrdcontext_to_json_v2_data *ctl, RRDCONTEXT
123 sizeof(struct alert_by_x_entry),
124 rcl);
125
126 - char *module = NULL;
127 - rrdlabels_get_value_strdup_or_null(st->rrdlabels, &module, "_collect_module");
128 - if(!module || !*module) module = "[unset]";
126 + char module[128];
127 + rrdlabels_get_value_strcpyz(st->rrdlabels, module, sizeof(module), "_collect_module");
128 + if(!*module)
129 + strncpyz(module, "[unset]", sizeof(module) - 1);
130
131 dictionary_set_advanced(ctl->alerts.by_module,
132 module,
src/database/rrd.c
+1
@@ -142,6 +142,7 @@ int rrd_init(const char *hostname, struct rrdhost_system_info *system_info, bool
142 , 1
143 , 0
144 );
145 + rrdhost_system_info_free(system_info);
146
147 if (unlikely(!localhost))
148 return 1;
src/database/rrdhost-system-info.c
+5
@@ -8,6 +8,11 @@
8 // coverity[ +tainted_string_sanitize_content : arg-0 ]
9 static inline void coverity_remove_taint(char *s __maybe_unused) { }
10
11 +void rrdhost_system_info_swap(struct rrdhost_system_info *a, struct rrdhost_system_info *b) {
12 + if(a && b)
13 + SWAP(*a, *b);
14 +}
15 +
16 // ----------------------------------------------------------------------------
17 // RRDHOST - set system info from environment variables
18 // system_info fields must be heap allocated or NULL
src/database/rrdhost-system-info.h
+1
@@ -100,5 +100,6 @@ void rrdhost_system_info_to_node_info(struct rrdhost_system_info *system_info, s
100 void rrdhost_system_info_to_streaming_function_array(BUFFER *wb, struct rrdhost_system_info *system_info);
101
102 void get_daemon_status_fields_from_system_info(DAEMON_STATUS_FILE *ds);
103 +void rrdhost_system_info_swap(struct rrdhost_system_info *a, struct rrdhost_system_info *b);
104
105 #endif //NETDATA_RRDHOST_SYSTEM_INFO_H
src/database/rrdhost.c
+5 -13
@@ -368,7 +368,8 @@ RRDHOST *rrdhost_create(
368
369 rrdhost_set_replication_parameters(host, memory_mode, replication_period, replication_step);
370
371 - host->system_info = system_info;
371 + host->system_info = rrdhost_system_info_create();
372 + rrdhost_system_info_swap(host->system_info, system_info);
373
374 rrdset_index_init(host);
375
@@ -543,12 +544,8 @@ static void rrdhost_update(RRDHOST *host
544
545 host->health.enabled = (mode == RRD_DB_MODE_NONE) ? 0 : health;
546
546 - {
547 - struct rrdhost_system_info *old = host->system_info;
548 - host->system_info = system_info;
549 - rrdhost_flag_set(host, RRDHOST_FLAG_METADATA_INFO | RRDHOST_FLAG_METADATA_CLAIMID | RRDHOST_FLAG_METADATA_UPDATE);
550 - rrdhost_system_info_free(old);
551 - }
547 + rrdhost_system_info_swap(host->system_info, system_info);
548 + rrdhost_flag_set(host, RRDHOST_FLAG_METADATA_INFO | RRDHOST_FLAG_METADATA_CLAIMID | RRDHOST_FLAG_METADATA_UPDATE);
549
550 rrdhost_init_os(host, os);
551 rrdhost_init_timezone(host, timezone, abbrev_timezone, utc_offset);
@@ -676,10 +673,8 @@ RRDHOST *rrdhost_find_or_create(
673 RRDHOST *host = rrdhost_find_by_guid(guid);
674 if (unlikely(host && host->rrd_memory_mode != mode && rrdhost_flag_check(host, RRDHOST_FLAG_ARCHIVED))) {
675
679 - if (likely(!archived && rrdhost_flag_check(host, RRDHOST_FLAG_PENDING_CONTEXT_LOAD))) {
680 - rrdhost_system_info_free(system_info);
676 + if (likely(!archived && rrdhost_flag_check(host, RRDHOST_FLAG_PENDING_CONTEXT_LOAD)))
677 return host;
682 - }
678
679 /* If a legacy memory mode instantiates all dbengine state must be discarded to avoid inconsistencies */
680 nd_log(NDLS_DAEMON, NDLP_INFO,
@@ -748,9 +743,6 @@ RRDHOST *rrdhost_find_or_create(
743 , system_info);
744 }
745
751 - if(!host)
752 - rrdhost_system_info_free(system_info);
753 -
746 return host;
747 }
748
src/database/sqlite/sqlite_aclk.c
+2
@@ -174,6 +174,8 @@ static int create_host_callback(void *data, int argc, char **argv, char **column
174 system_info,
175 1);
176
177 + rrdhost_system_info_free(system_info);
178 +
179 if (likely(host)) {
180 if (is_ephemeral)
181 rrdhost_option_set(host, RRDHOST_OPTION_EPHEMERAL_HOST);
src/plugins.d/pluginsd_parser.c
+5 -1
@@ -194,6 +194,8 @@ static inline PARSER_RC pluginsd_host_define_end(char **words __maybe_unused, si
194 if(!parser->user.host_define.parsing_host)
195 return PLUGINSD_DISABLE_PLUGIN(parser, PLUGINSD_KEYWORD_HOST_DEFINE_END, "missing initialization, send " PLUGINSD_KEYWORD_HOST_DEFINE " before this");
196
197 + struct rrdhost_system_info *system_info = rrdhost_system_info_from_host_labels(parser->user.host_define.rrdlabels);
198 +
199 RRDHOST *host = rrdhost_find_or_create(
200 string2str(parser->user.host_define.hostname),
201 string2str(parser->user.host_define.hostname),
@@ -215,9 +217,11 @@ static inline PARSER_RC pluginsd_host_define_end(char **words __maybe_unused, si
217 stream_receive.replication.enabled,
218 stream_receive.replication.period,
219 stream_receive.replication.step,
218 - rrdhost_system_info_from_host_labels(parser->user.host_define.rrdlabels),
220 + system_info,
221 false);
222
223 + rrdhost_system_info_free(system_info);
224 +
225 rrdhost_option_set(host, RRDHOST_OPTION_VIRTUAL_HOST);
226 rrdhost_flag_set(host, RRDHOST_FLAG_COLLECTOR_ONLINE);
227 object_state_activate_if_not_activated(&host->state_id);
src/streaming/stream-receiver-connection.c
+2 -1
@@ -173,7 +173,8 @@ static bool stream_receiver_send_first_response(struct receiver_state *rpt) {
173 rpt->config.replication.step,
174 rpt->system_info,
175 0);
176 - // IMPORTANT: system_info is now consumed!
176 +
177 + rrdhost_system_info_free(rpt->system_info);
178 rpt->system_info = NULL;
179
180 if(!host) {