@cryptotaxi247 / netdata-1 / commits / 82fcb4f4c

Fix ARAL double-free race (#22294)

* ARAL: close double-free race by atomically claiming the slot trailer aral_freez_internal() used a non-atomic load+store on the trailer. Two concurrent double-frees of the same pointer could both see the same page and enqueue the slot onto the free list twice, leading to later trailer corruption and crashes (e.g. in aral_unmark_allocation following a stale page pointer). Replace with __atomic_exchange_n(.., 0, ACQ_REL); the loser sees NULL and fatal()s with a clear diagnostic before touching the free list. aral_unmark_allocation() has the same load-then-modify shape but is not exercised concurrently by any current caller; left for follow-up. * ARAL: short-circuit claim on NULL trailer; broaden free fatal message Have aral_claim_page_pointer_after_element_*() return NULL directly when the atomic exchange yields 0, so the freez path's explicit fatal() runs in both debug and release builds (was previously preempted by aral_decode_*'s internal_fatal in NETDATA_INTERNAL_CHECKS builds). The load path keeps its debug-only NULL assertion unchanged. Reword the fatal to also cover memory corruption / invalid pointers that happen to have 0 in the trailer.

Stelios Fragkakis committed Apr 27, 2026 at 21:41 UTC 82fcb4f4cc466a16854f520de9753bb8b7662b5d

1 file changed +32 -12

src/libnetdata/aral/aral.c

+32 -12

@@ -431,11 +431,7 @@ static inline ARAL_PAGE *find_page_with_allocation_internal_check(ARAL *ar, void

431 // --------------------------------------------------------------------------------------------------------------------

432 // Tagging the pointer with the 'marked' flag

433

434 -// Retrieving the pointer and the 'marked' flag

435 -static ALWAYS_INLINE ARAL_PAGE *aral_get_page_pointer_after_element___do_NOT_have_aral_lock(ARAL *ar, void *ptr, bool *marked) {

436 - uint8_t *data = ptr;

437 - uintptr_t *page_ptr = (uintptr_t *)&data[ar->config.element_ptr_offset];

438 - uintptr_t tagged_page = __atomic_load_n(page_ptr, __ATOMIC_ACQUIRE); // Atomically load the tagged pointer

434 +static ALWAYS_INLINE ARAL_PAGE *aral_decode_page_pointer_after_element___do_NOT_have_aral_lock(ARAL *ar, void *ptr, uintptr_t tagged_page, bool *marked) {

435 *marked = (tagged_page & 1) != 0; // Extract the LSB as the 'marked' flag

436 ARAL_PAGE *page = (ARAL_PAGE *)(tagged_page & ~1); // Mask out the LSB to get the original pointer

437

@@ -468,6 +464,31 @@ static ALWAYS_INLINE ARAL_PAGE *aral_get_page_pointer_after_element___do_NOT_hav

464 return page;

465 }

466

467 +// Retrieving the pointer and the 'marked' flag

468 +static ALWAYS_INLINE ARAL_PAGE *aral_get_page_pointer_after_element___do_NOT_have_aral_lock(ARAL *ar, void *ptr, bool *marked) {

469 + uint8_t *data = ptr;

470 + uintptr_t *page_ptr = (uintptr_t *)&data[ar->config.element_ptr_offset];

471 + uintptr_t tagged_page = __atomic_load_n(page_ptr, __ATOMIC_ACQUIRE); // Atomically load the tagged pointer

472 +

473 + return aral_decode_page_pointer_after_element___do_NOT_have_aral_lock(ar, ptr, tagged_page, marked);

474 +}

475 +

476 +// Atomically claims an allocated slot for freeing.

477 +// Returns NULL on a concurrent double-free or stale free, leaving the

478 +// decode helper's NULL assertion to the load path only.

479 +static ALWAYS_INLINE ARAL_PAGE *aral_claim_page_pointer_after_element___do_NOT_have_aral_lock(ARAL *ar, void *ptr, bool *marked) {

480 + uint8_t *data = ptr;

481 + uintptr_t *page_ptr = (uintptr_t *)&data[ar->config.element_ptr_offset];

482 + uintptr_t tagged_page = __atomic_exchange_n(page_ptr, 0, __ATOMIC_ACQ_REL);

483 +

484 + if(unlikely(!tagged_page)) {

485 + *marked = false;

486 + return NULL;

487 + }

488 +

489 + return aral_decode_page_pointer_after_element___do_NOT_have_aral_lock(ar, ptr, tagged_page, marked);

490 +}

491 +

492 static ALWAYS_INLINE void aral_set_page_pointer_after_element___do_NOT_have_aral_lock(ARAL *ar, void *page, void *ptr, bool marked) {

493 uint8_t *data = ptr;

494 uintptr_t *page_ptr = (uintptr_t *)&data[ar->config.element_ptr_offset];

@@ -1036,14 +1057,13 @@ void aral_freez_internal(ARAL *ar, void *ptr TRACE_ALLOCATIONS_FUNCTION_DEFINITI

1057

1058 if(unlikely(!ptr)) return;

1059

1039 - // get the page pointer

1060 + // Atomically claim the trailer: the losing thread of a concurrent

1061 + // double-free observes a NULL pointer and fatal()s here, while only the

1062 + // winner enqueues the slot back to the free list.

1063 bool marked;

1041 - ARAL_PAGE *page = aral_get_page_pointer_after_element___do_NOT_have_aral_lock(ar, ptr, &marked);

1042 -

1043 - // Clear the embedded trailer before publishing the slot back to the free lists.

1044 - // This makes stale/double frees fail through the null-page check instead of

1045 - // dereferencing a stale ARAL_PAGE pointer while the slot is idle.

1046 - aral_set_page_pointer_after_element___do_NOT_have_aral_lock(ar, NULL, ptr, false);

1064 + ARAL_PAGE *page = aral_claim_page_pointer_after_element___do_NOT_have_aral_lock(ar, ptr, &marked);

1065 + if(unlikely(!page))

1066 + fatal("ARAL: '%s' double free, stale free, or corrupted pointer %p", ar->config.name, ptr);

1067

1068 size_t idx = mark_to_idx(marked);

1069 __atomic_add_fetch(&ar->ops[idx].atomic.deallocators, 1, __ATOMIC_RELAXED);