Fix use after free in metric registry (#20947)
Mark for deferred deletion
Stelios Fragkakis committed
Sep 9, 2025 at 20:34 UTC
387c400d4e5a86a87e5ea0981d2d4f2a68120a8f
1 file changed
+25
-15
src/database/engine/mrg-internals.h
+25
-15
@@ -14,6 +14,7 @@ struct metric {
14
15
REFCOUNT refcount;
16
uint8_t partition;
17
+ bool deleted;
18
19
uint32_t latest_update_every_s; // the latest data collection frequency
20
@@ -198,8 +199,9 @@ static void acquired_for_deletion_metric_delete(MRG *mrg, METRIC *metric) {
199
MRG_STATS_DELETED_METRIC(mrg, partition, metric->section);
200
201
mrg_index_write_unlock(mrg, partition);
201
- uuidmap_free(metric->uuid);
202
- aral_freez(mrg->index[partition].aral, metric);
202
+
203
+ __atomic_store_n(&metric->deleted, true, __ATOMIC_RELEASE);
204
+
205
mrg_stats_judy_mem(mrg, partition, JudyAllocThreadPulseGetAndReset());
206
}
207
@@ -209,6 +211,11 @@ static bool metric_acquire(MRG *mrg, METRIC *metric) {
211
if(!REFCOUNT_ACQUIRED(rc))
212
return false;
213
214
+ if (__atomic_load_n(&metric->deleted, __ATOMIC_ACQUIRE)) {
215
+ refcount_release(&metric->refcount);
216
+ return false;
217
+ }
218
+
219
size_t partition = metric->partition;
220
221
if(rc == 1)
@@ -223,21 +230,23 @@ ALWAYS_INLINE
230
static bool metric_release(MRG *mrg, METRIC *metric) {
231
size_t partition = metric->partition;
232
226
- REFCOUNT refcount = refcount_release(&metric->refcount);
227
-
228
- if(!refcount && !acquired_metric_has_retention(mrg, metric) && refcount_acquire_for_deletion(&metric->refcount))
229
- refcount = REFCOUNT_DELETED;
230
-
231
- if(refcount == 0 || refcount == REFCOUNT_DELETED) {
232
- __atomic_sub_fetch(&mrg->index[partition].stats.entries_acquired, 1, __ATOMIC_RELAXED);
233
-
234
- if(refcount == REFCOUNT_DELETED)
235
- acquired_for_deletion_metric_delete(mrg, metric);
233
+ if (refcount_release(&metric->refcount) == 0) {
234
+ // we are the last user
235
+ bool already_deleted = __atomic_load_n(&metric->deleted, __ATOMIC_ACQUIRE);
236
+ if (already_deleted || !acquired_metric_has_retention(mrg, metric)) {
237
+ if (!already_deleted) {
238
+ acquired_for_deletion_metric_delete(mrg, metric);
239
+ }
240
+ uuidmap_free(metric->uuid);
241
+ aral_freez(mrg->index[partition].aral, metric);
242
+ __atomic_sub_fetch(&mrg->index[partition].stats.entries_acquired, 1, __ATOMIC_RELAXED);
243
+ __atomic_sub_fetch(&mrg->index[partition].stats.current_references, 1, __ATOMIC_RELAXED);
244
+ return true;
245
+ }
246
}
247
248
__atomic_sub_fetch(&mrg->index[partition].stats.current_references, 1, __ATOMIC_RELAXED);
239
-
240
- return refcount == REFCOUNT_DELETED;
249
+ return false;
250
}
251
252
ALWAYS_INLINE
@@ -293,6 +302,7 @@ static METRIC *metric_add_and_acquire(MRG *mrg, MRG_ENTRY *entry, bool *ret) {
302
metric->latest_time_s_clean = MAX(0, entry->last_time_s);
303
metric->latest_time_s_hot = 0;
304
metric->latest_update_every_s = entry->latest_update_every_s;
305
+ metric->deleted = false;
306
#ifdef NETDATA_INTERNAL_CHECKS
307
metric->writer = 0;
308
#endif
@@ -349,4 +359,4 @@ static METRIC *metric_get_and_acquire_by_id(MRG *mrg, UUIDMAP_ID id, Word_t sect
359
}
360
}
361
352
-#endif //NETDATA_MRG_INTERNALS_H
362
+#endif //NETDATA_MRG_INTERNALS_H
\ No newline at end of file