@cryptotaxi247 / netdata-1 / commits / 65c43f455

Fix memory ordering for aclk_host_config publish (#22429)

* fix(aclk): close publish-before-init race on host->aclk_host_config create_aclk_config() initializes host, stream_alerts, and node_info_send_time before the CAS that publishes the pointer; the CAS uses __ATOMIC_RELEASE on success. Every host->aclk_host_config reader is upgraded to __ATOMIC_ACQUIRE so the pairing holds on weakly-ordered hardware. Closes a NULL-deref vector at sqlite_aclk_alert.c:996 where schedule_alert_snapshot_if_needed() can deref aclk_host_config->host while another worker thread is still in create_aclk_config(). * fix(aclk): resolve race condition in aclk_host_config initialization

Stelios Fragkakis committed May 7, 2026 at 08:52 UTC 65c43f455c58bdd09120ebfec7913fbb23d5c7d2
9 files changed +39 -36
src/aclk/aclk.c
+2 -2
@@ -1052,7 +1052,7 @@ void aclk_send_bin_msg(char *msg, size_t msg_len, enum aclk_topics subtopic, con
1052
1053 static void fill_alert_status_for_host(BUFFER *wb, RRDHOST *host)
1054 {
1055 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
1055 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
1056 if (!aclk_host_config)
1057 return;
1058
@@ -1171,7 +1171,7 @@ char *aclk_state(void)
1171
1172 static void fill_alert_status_for_host_json(json_object *obj, RRDHOST *host)
1173 {
1174 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
1174 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
1175 if (!aclk_host_config)
1176 return;
1177
src/database/contexts/rrdcontext.c
+2 -2
@@ -246,7 +246,7 @@ static bool rrdcontext_checkpoint_generation_is_current(RRDHOST *host, const cha
246 // Save a pending checkpoint to be replayed when context processing completes.
247 // Returns true if saved successfully, false if save failed (caller should execute immediately).
248 static bool rrdcontext_checkpoint_save_pending(RRDHOST *host, struct ctxs_checkpoint *cmd) {
249 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
249 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
250 if(!aclk_host_config)
251 return false;
252
@@ -439,7 +439,7 @@ void rrdcontext_hub_stop_streaming_command(void *ptr) {
439 #define PENDING_CTX_CHECKPOINT_MAX_AGE_S 300
440
441 void rrdcontext_hub_pending_checkpoint_replay(RRDHOST *host) {
442 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
442 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
443 if(!aclk_host_config || !__atomic_load_n(&aclk_host_config->pending_ctx_checkpoint, __ATOMIC_ACQUIRE))
444 return;
445
src/database/sqlite/sqlite_aclk.c
+17 -14
@@ -697,10 +697,10 @@ static void aclk_synchronization_event_loop(void *arg)
697 // NODE STATE
698 case ACLK_DATABASE_NODE_STATE:
699 host = cmd.param[0];
700 - aclk_host_config = host->aclk_host_config;
700 + aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
701 if (unlikely(!aclk_host_config)) {
702 create_aclk_config(host, &host->host_id.uuid, &host->node_id.uuid);
703 - aclk_host_config = host->aclk_host_config;
703 + aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
704 }
705
706 if (aclk_host_config) {
@@ -730,17 +730,17 @@ static void aclk_synchronization_event_loop(void *arg)
730 case ACLK_QUEUE_NODE_INFO:
731 host = cmd.param[0];
732 bool immediate = (bool)(uintptr_t)cmd.param[1];
733 - aclk_host_config = host->aclk_host_config;
733 + aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
734 if (unlikely(!aclk_host_config)) {
735 create_aclk_config(host, &host->host_id.uuid, &host->node_id.uuid);
736 - aclk_host_config = host->aclk_host_config;
736 + aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
737 }
738 aclk_host_config->node_info_send_time = (host == localhost || immediate) ? 1 : now_realtime_sec();
739 break;
740 case ACLK_CANCEL_NODE_UPDATE_TIMER:
741 host = cmd.param[0];
742 struct completion *compl = cmd.param[1];
743 - aclk_host_config = host->aclk_host_config;
743 + aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
744 if (!aclk_host_config || !aclk_host_config->timer_initialized) {
745 completion_mark_complete(compl);
746 break;
@@ -988,7 +988,7 @@ static void aclk_initialize_event_loop(void)
988 void create_aclk_config(RRDHOST *host, nd_uuid_t *host_uuid __maybe_unused, nd_uuid_t *node_id __maybe_unused)
989 {
990
991 - if (!host || host->aclk_host_config)
991 + if (!host || __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE))
992 return;
993
994 struct aclk_sync_cfg_t *aclk_host_config = callocz(1, sizeof(struct aclk_sync_cfg_t));
@@ -996,8 +996,16 @@ void create_aclk_config(RRDHOST *host, nd_uuid_t *host_uuid __maybe_unused, nd_u
996 if (node_id && !uuid_is_null(*node_id))
997 uuid_unparse_lower(*node_id, aclk_host_config->node_id);
998
999 + // Initialize every field BEFORE publishing the pointer via CAS; the RELEASE on
1000 + // the CAS pairs with ACQUIRE loads of host->aclk_host_config in readers so they
1001 + // cannot observe the pointer with zero-initialized fields (host == NULL etc.).
1002 + aclk_host_config->host = host;
1003 + aclk_host_config->stream_alerts = false;
1004 + time_t now = now_realtime_sec();
1005 + aclk_host_config->node_info_send_time = (host == localhost || NULL == localhost) ? now - 25 : now;
1006 +
1007 struct aclk_sync_cfg_t *expected = NULL;
1000 - if (__atomic_compare_exchange_n(&host->aclk_host_config, &expected, aclk_host_config, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
1008 + if (__atomic_compare_exchange_n(&host->aclk_host_config, &expected, aclk_host_config, false, __ATOMIC_RELEASE, __ATOMIC_RELAXED)) {
1009 if (node_id && UUIDiszero(host->node_id))
1010 uuid_copy(host->node_id.uuid, *node_id);
1011 }
@@ -1005,11 +1013,6 @@ void create_aclk_config(RRDHOST *host, nd_uuid_t *host_uuid __maybe_unused, nd_u
1013 freez(aclk_host_config);
1014 return;
1015 }
1008 -
1009 - aclk_host_config->host = host;
1010 - aclk_host_config->stream_alerts = false;
1011 - time_t now = now_realtime_sec();
1012 - aclk_host_config->node_info_send_time = (host == localhost || NULL == localhost) ? now - 25 : now;
1016 }
1017
1018 #define SQL_FETCH_ALL_HOSTS \
@@ -1208,7 +1211,7 @@ void destroy_aclk_config(RRDHOST *host)
1211 if (!host)
1212 return;
1213
1211 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
1214 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
1215 if (!aclk_host_config)
1216 return;
1217
@@ -1221,7 +1224,7 @@ void destroy_aclk_config(RRDHOST *host)
1224 completion_destroy(&compl);
1225 }
1226
1224 - struct aclk_sync_cfg_t *old_aclk_host_config = __atomic_exchange_n(&host->aclk_host_config, NULL, __ATOMIC_RELAXED);
1227 + struct aclk_sync_cfg_t *old_aclk_host_config = __atomic_exchange_n(&host->aclk_host_config, NULL, __ATOMIC_ACQUIRE);
1228 if (!old_aclk_host_config)
1229 return;
1230
src/database/sqlite/sqlite_aclk_alert.c
+8 -8
@@ -514,7 +514,7 @@ static void aclk_push_alert_event(RRDHOST *host, sqlite3_stmt **res, sqlite3_stm
514
515 param = 0;
516 RRDCALC_STATUS status;
517 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
517 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
518 while (sqlite3_step_monitored(*res) == SQLITE_ROW) {
519 health_alarm_log_populate(&alarm_log, *res, host, &status);
520 aclk_send_alarm_log_entry(&alarm_log);
@@ -669,7 +669,7 @@ bool process_alert_pending_queue(RRDHOST *host)
669 RRDCALC_STATUS new_status = sqlite3_column_int(res, 2);
670 int64_t row = sqlite3_column_int64(res, 3);
671
672 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
672 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
673 if (aclk_host_config) {
674 int ret = insert_alert_to_submit_queue(host, health_log_id, unique_id, new_status);
675 if (ret == 0)
@@ -705,7 +705,7 @@ void aclk_push_alert_events_for_all_hosts(void)
705
706 rrdhost_flag_clear(host, RRDHOST_FLAG_ACLK_STREAM_ALERTS);
707
708 - struct aclk_sync_cfg_t *aclk_host_config = host->aclk_host_config;
708 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
709 if (!aclk_host_config || false == aclk_host_config->stream_alerts || rrdhost_flag_check(host, RRDHOST_FLAG_ARCHIVED)) {
710 (void)process_alert_pending_queue(host);
711 commit_alert_events(host);
@@ -807,7 +807,7 @@ void aclk_send_alert_configuration(char *config_hash)
807 if (unlikely(!config_hash))
808 return;
809
810 - struct aclk_sync_cfg_t *aclk_host_config = localhost->aclk_host_config;
810 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&localhost->aclk_host_config, __ATOMIC_ACQUIRE);
811
812 if (unlikely(!aclk_host_config))
813 return;
@@ -834,7 +834,7 @@ void aclk_push_alert_config_event(char *node_id __maybe_unused, char *config_has
834
835 RRDHOST *host = rrdhost_find_by_node_id(node_id);
836
837 - if (!host || !(aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED))) {
837 + if (!host || !(aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE))) {
838 freez(config_hash);
839 freez(node_id);
840 return;
@@ -1051,7 +1051,7 @@ done:
1051 #define ALARM_EVENTS_PER_CHUNK 1000
1052 void send_alert_snapshot_to_cloud(RRDHOST *host __maybe_unused)
1053 {
1054 - struct aclk_sync_cfg_t *aclk_host_config = host->aclk_host_config;
1054 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
1055
1056 if (unlikely(!host)) {
1057 nd_log(NDLS_ACCESS, NDLP_WARNING, "AC [%s (N/A)]: Node id not found", aclk_host_config->node_id);
@@ -1157,7 +1157,7 @@ void aclk_start_alert_streaming(char *node_id, uint64_t cloud_version)
1157 struct aclk_sync_cfg_t *aclk_host_config;
1158 RRDHOST *host = rrdhost_find_by_node_id(node_id);
1159
1160 - if (!host || !(aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED))) {
1160 + if (!host || !(aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE))) {
1161 nd_log(NDLS_ACCESS, NDLP_NOTICE, "ACLK STA [%s (N/A)]: Ignoring request to stream alert state changes, invalid node.", node_id);
1162 return;
1163 }
@@ -1192,7 +1192,7 @@ void aclk_alert_version_check(char *node_id, char *claim_id, uint64_t cloud_vers
1192 struct aclk_sync_cfg_t *aclk_host_config;
1193 RRDHOST *host = rrdhost_find_by_node_id(node_id);
1194
1195 - if (!host || !(aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED)))
1195 + if (!host || !(aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE)))
1196 nd_log(NDLS_ACCESS, NDLP_NOTICE,
1197 "ACLK REQ [%s (N/A)]: ALERTS CHECKPOINT VALIDATION REQUEST RECEIVED FOR INVALID NODE",
1198 node_id);
src/database/sqlite/sqlite_aclk_node.c
+5 -5
@@ -26,7 +26,7 @@ DICTIONARY *collectors_from_charts(RRDHOST *host, DICTIONARY *dict) {
26
27 static void build_node_collectors(RRDHOST *host)
28 {
29 - struct aclk_sync_cfg_t *aclk_host_config = host->aclk_host_config;
29 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
30
31 struct update_node_collectors upd_node_collectors;
32 DICTIONARY *dict = dictionary_create(DICT_OPTION_SINGLE_THREADED);
@@ -49,7 +49,7 @@ static void build_node_info(RRDHOST *host, struct aclk_sync_completion *sync_com
49 {
50 struct update_node_info node_info;
51
52 - struct aclk_sync_cfg_t *aclk_host_config = host->aclk_host_config;
52 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
53
54 CLAIM_ID claim_id = claim_id_get();
55
@@ -105,7 +105,7 @@ static void build_node_info(RRDHOST *host, struct aclk_sync_completion *sync_com
105
106 void send_node_info_with_wait(RRDHOST *host)
107 {
108 - if (unlikely(!host || !__atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED)))
108 + if (unlikely(!host || !__atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE)))
109 return;
110
111 // No node_id means cloud doesn't know about this node - nothing to update
@@ -130,7 +130,7 @@ void send_node_info_with_wait(RRDHOST *host)
130
131 void send_node_update_with_wait(RRDHOST *host, int live, int queryable)
132 {
133 - if (unlikely(!host || !__atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED)))
133 + if (unlikely(!host || !__atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE)))
134 return;
135
136 // No node_id means cloud doesn't know about this node - nothing to update
@@ -177,7 +177,7 @@ void aclk_check_node_info_and_collectors(void)
177 time_t now = now_realtime_sec();
178 dfe_start_reentrant(rrdhost_root_index, host)
179 {
180 - struct aclk_sync_cfg_t *aclk_host_config = host->aclk_host_config;
180 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
181 if (unlikely(!aclk_host_config))
182 continue;
183
src/database/sqlite/sqlite_health.c
+1 -1
@@ -183,7 +183,7 @@ static void insert_alert_queue(
183
184 int rc;
185
186 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
186 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
187 if (!aclk_host_config)
188 return;
189
src/database/sqlite/sqlite_metadata.c
+1 -1
@@ -275,7 +275,7 @@ static inline void set_host_node_id(RRDHOST *host, nd_uuid_t *node_id)
275 return;
276 }
277
278 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
278 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
279
280 uuid_copy(host->node_id.uuid, *node_id);
281
src/health/health_event_loop.c
+2 -2
@@ -370,7 +370,7 @@ static void health_event_loop_for_host(RRDHOST *host, bool apply_hibernation_del
370
371 worker_is_busy(WORKER_HEALTH_JOB_HOST_LOCK);
372 {
373 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
373 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
374 if (aclk_host_config && aclk_host_config->send_snapshot == 2)
375 return;
376 }
@@ -751,7 +751,7 @@ static void health_event_loop_for_host(RRDHOST *host, bool apply_hibernation_del
751 commit_alert_transitions(host);
752
753 if (!__atomic_load_n(&host->health.pending_transitions, __ATOMIC_RELAXED)) {
754 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
754 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
755 if (aclk_host_config && aclk_host_config->send_snapshot == 1) {
756 aclk_host_config->send_snapshot = 2;
757 rrdhost_flag_set(host, RRDHOST_FLAG_ACLK_STREAM_ALERTS);
src/plugins.d/pluginsd_parser.c
+1 -1
@@ -262,7 +262,7 @@ static inline PARSER_RC pluginsd_host_define_end(char **words __maybe_unused, si
262
263 rrdhost_flag_clear(host, RRDHOST_FLAG_ORPHAN);
264 rrdcontext_host_child_connected(host);
265 - struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_RELAXED);
265 + struct aclk_sync_cfg_t *aclk_host_config = __atomic_load_n(&host->aclk_host_config, __ATOMIC_ACQUIRE);
266 if (aclk_host_config)
267 aclk_queue_node_info(host, true);
268 else