@cryptotaxi247 / netdata / commits / e8caf8064

Fix aral race condition (#22367)

* aral: serialize unmark/freez via UNMARKING trailer state, prevent counter race and page-lifetime UAF Introduce a third trailer state (page | UNMARKING) that aral_unmark_allocation publishes before taking the page lock to decrement marked_elements, and clears after. While the bit is visible, aral_freez_internal observes it in its claim and spins until the final UNMARKED state is published, so: - freez never sees an unmarked trailer with a stale marked_elements counter (no spurious "marked > used" assertion under NETDATA_INTERNAL_CHECKS) - the slot's refcount contribution stays in place across the trailer transition, so the page cannot be destroyed under unmark's page_lock (closes the UAF that surfaced in production as gorilla_writer_aral_unmark SEGVs and aral_set_page_pointer dereferences of stale page pointers) The freez hot path keeps its single atomic-exchange; the cold path (UNMARKING observed) restores the bit and retries. Bit availability is guarded with a _Static_assert against SYSTEM_REQUIRED_ALIGNMENT. Add aral_unittest_concurrency() (8 scenarios) wired into aral_unittest under NETDATA_INTERNAL_CHECKS and reachable via -W aralconcurrency: 1-3 clean unmark / unmark-on-unmarked / clean freez (with marked guard on the same page) 4-6 forced races: freez wins claim, unmark wins trailer transition, and the same with no grace period - each verifies counter consistency 7 last marked on page triggers list move 8 coordinated stress: 256 pointers, deterministic UNMARKING for every slot, asserts the cold path was entered via a counter incremented in aral_claim_page_pointer_after_element___wait_for_unmark * Address review comments * Address review comments (2) * Address review comments (3) * Address review comments (4) * Update src/libnetdata/aral/aral.c Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Address review comments (5) * Address review comments (6) * Address review comments (7) --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Stelios Fragkakis committed May 3, 2026 at 18:16 UTC e8caf8064d068a977840e55ee46737764a08e3ca

3 files changed +740 -28

src/daemon/main.c

@@ -479,6 +479,15 @@ int netdata_main(int argc, char **argv) {

479 unittest_running = true;

480 return aral_unittest(10000);

481 }

482 + else if(strcmp(optarg, "aralconcurrency") == 0) {

483 + unittest_running = true;

484 +#ifdef NETDATA_INTERNAL_CHECKS

485 + return aral_unittest_concurrency();

486 +#else

487 + fprintf(stderr, "aralconcurrency requires NETDATA_INTERNAL_CHECKS\n");

488 + return 1;

489 +#endif

490 + }

491 else if(strcmp(optarg, "waitqtest") == 0) {

492 unittest_running = true;

493 return unittest_waiting_queue();

src/libnetdata/aral/aral.c

+722 -26

@@ -303,6 +303,69 @@ static ALWAYS_INLINE void aral_unittest_wait_for_race_window(ARAL *ar, ARAL_PAGE

303 }

304 }

305 }

306 +

307 +// Pause-point hook for the unmark/freez state-machine concurrency tests.

308 +// Distinct from aral_race_unittest_hook above; fires only on a (ar, ptr, stage)

309 +// match so the two test families do not interfere.

310 +enum aral_concurrency_race_stage {

311 + ARAL_CONCURRENCY_RACE_NONE = 0,

312 + ARAL_CONCURRENCY_RACE_UNMARK_BEFORE_CAS, // unmark entry, before reading the trailer

313 + ARAL_CONCURRENCY_RACE_UNMARK_AFTER_CAS, // unmark after CAS to UNMARKING, before page_lock

314 + ARAL_CONCURRENCY_RACE_FREEZ_BEFORE_CLAIM, // freez entry, before atomic-exchange

315 +};

316 +

317 +struct aral_concurrency_race_hook {

318 + ARAL *ar;

319 + void *target_ptr;

320 + enum aral_concurrency_race_stage stage;

321 + bool enabled;

322 + bool waiting;

323 + bool release;

324 +};

325 +

326 +static struct aral_concurrency_race_hook aral_concurrency_race_hook = { 0 };

327 +

328 +// Counter incremented every time aral_claim_page_pointer_after_element___wait_for_unmark

329 +// enters the cold path (i.e. observed UNMARKING in the trailer). Tests poll

330 +// this to verify they actually exercised the cold path before completing.

331 +static size_t aral_freez_unmarking_observed_count = 0;

332 +

333 +// Arm the concurrency race hook for a (ar, ptr, stage) match.

334 +// Sets the match fields first, then publishes `enabled = true` with a

335 +// release-store. Paired with the acquire-load in aral_concurrency_race_pause,

336 +// this guarantees a reader that observes enabled==true sees fully-published

337 +// match fields. Without this ordering the compiler is free to publish

338 +// `enabled` before the other fields, causing the pause to miss.

339 +static inline void aral_concurrency_race_hook_arm(ARAL *ar, void *target_ptr,

340 + enum aral_concurrency_race_stage stage) {

341 + aral_concurrency_race_hook.ar = ar;

342 + aral_concurrency_race_hook.target_ptr = target_ptr;

343 + aral_concurrency_race_hook.stage = stage;

344 + aral_concurrency_race_hook.waiting = false;

345 + aral_concurrency_race_hook.release = false;

346 + __atomic_store_n(&aral_concurrency_race_hook.enabled, true, __ATOMIC_RELEASE);

347 +}

348 +

349 +// Reset the hook. Callers should only invoke this after every concurrent

350 +// reader is known to have stopped (i.e. all racing threads have been joined).

351 +static inline void aral_concurrency_race_hook_reset(void) {

352 + __atomic_store_n(&aral_concurrency_race_hook.enabled, false, __ATOMIC_RELEASE);

353 + aral_concurrency_race_hook = (struct aral_concurrency_race_hook){ 0 };

354 +}

355 +

356 +static ALWAYS_INLINE void aral_concurrency_race_pause(ARAL *ar, void *ptr, enum aral_concurrency_race_stage stage) {

357 + // Acquire-load on `enabled` pairs with the release-store in

358 + // aral_concurrency_race_hook_arm so that if we observe enabled==true,

359 + // the other match fields are fully published.

360 + if(unlikely(__atomic_load_n(&aral_concurrency_race_hook.enabled, __ATOMIC_ACQUIRE) &&

361 + aral_concurrency_race_hook.ar == ar &&

362 + aral_concurrency_race_hook.target_ptr == ptr &&

363 + aral_concurrency_race_hook.stage == stage)) {

364 + __atomic_store_n(&aral_concurrency_race_hook.waiting, true, __ATOMIC_RELEASE);

365 + while(!__atomic_load_n(&aral_concurrency_race_hook.release, __ATOMIC_ACQUIRE))

366 + tinysleep();

367 + }

368 +}

369 #endif

370

371 static ALWAYS_INLINE bool aral_adders_trylock(ARAL *ar, bool marked) {

@@ -430,10 +493,37 @@ static inline ARAL_PAGE *find_page_with_allocation_internal_check(ARAL *ar, void

493

494 // --------------------------------------------------------------------------------------------------------------------

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

496 +//

497 +// Trailer state machine (the low bits of the per-slot trailer word):

498 +// 0 = freed / on the free list

499 +// page = allocated, unmarked

500 +// page | ARAL_TRAILER_MARKED = allocated, marked

501 +// page | ARAL_TRAILER_UNMARKING = allocated, unmark in progress

502 +// (page counters not yet updated)

503 +//

504 +// Page pointers must be aligned such that the low 2 bits are always 0,

505 +// otherwise the tag bits would collide with real address bits. The static

506 +// assert below enforces this at compile time on every supported platform.

507 +//

508 +// The UNMARKING state is set by aral_unmark_allocation() before it takes the

509 +// page lock to decrement page->page_lock.marked_elements, and is cleared

510 +// after the decrement is complete (still under page lock). While UNMARKING

511 +// is visible, aral_freez_internal() observes it and waits, so:

512 +// - the slot's refcount contribution remains in place, keeping the page

513 +// alive across our trailer transition (no UAF on aral_page_lock())

514 +// - freez never sees an unmarked trailer with a stale marked_elements

515 +// counter (no spurious "marked > used" assertion)

516 +

517 +#define ARAL_TRAILER_MARKED ((uintptr_t)0x1)

518 +#define ARAL_TRAILER_UNMARKING ((uintptr_t)0x2)

519 +#define ARAL_TRAILER_TAG_MASK (ARAL_TRAILER_MARKED | ARAL_TRAILER_UNMARKING)

520 +

521 +_Static_assert((SYSTEM_REQUIRED_ALIGNMENT & ARAL_TRAILER_TAG_MASK) == 0,

522 + "ARAL trailer tag bits collide with page pointer alignment");

523

524 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

525 + *marked = (tagged_page & ARAL_TRAILER_MARKED) != 0;

526 + ARAL_PAGE *page = (ARAL_PAGE *)(tagged_page & ~ARAL_TRAILER_TAG_MASK);

527

528 internal_fatal(!page,

529 "ARAL: '%s' possible corruption or double free of pointer %p",

@@ -474,27 +564,52 @@ static ALWAYS_INLINE ARAL_PAGE *aral_get_page_pointer_after_element___do_NOT_hav

564 }

565

566 // 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) {

567 +//

568 +// Hot path: single atomic-exchange to 0 (same primitive as before the unmark

569 +// protocol existed - no extra load, no CAS).

570 +//

571 +// Cold path: if the exchange returned a value with UNMARKING set, we

572 +// accidentally claimed a slot mid-unmark. Best-effort restore the UNMARKING

573 +// state so unmark can finish, yield briefly, and retry. The restore CAS only

574 +// succeeds if the trailer is still 0 (i.e. nothing else touched it since our

575 +// exchange); if it fails, the next iteration's exchange picks up whatever

576 +// settled value the other writer left behind.

577 +//

578 +// Returns NULL on a concurrent double-free or stale free.

579 +static ALWAYS_INLINE ARAL_PAGE *aral_claim_page_pointer_after_element___wait_for_unmark(ARAL *ar, void *ptr, bool *marked) {

580 uint8_t *data = ptr;

581 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);

582

484 - if(unlikely(!tagged_page)) {

485 - *marked = false;

486 - return NULL;

487 - }

583 + while(true) {

584 + uintptr_t prior = __atomic_exchange_n(page_ptr, 0, __ATOMIC_ACQ_REL);

585

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

586 + if(unlikely(!prior)) {

587 + *marked = false;

588 + return NULL;

589 + }

590 +

591 + if(likely(!(prior & ARAL_TRAILER_UNMARKING)))

592 + return aral_decode_page_pointer_after_element___do_NOT_have_aral_lock(ar, ptr, prior, marked);

593 +

594 + // Cold path: we exchanged with the UNMARKING transition. Put it back

595 + // (only succeeds if the trailer is still 0) and retry once unmark has

596 + // had a chance to publish the final state.

597 + uintptr_t zero = 0;

598 + __atomic_compare_exchange_n(page_ptr, &zero, prior,

599 + false, __ATOMIC_RELEASE, __ATOMIC_RELAXED);

600 +#ifdef NETDATA_INTERNAL_CHECKS

601 + __atomic_add_fetch(&aral_freez_unmarking_observed_count, 1, __ATOMIC_RELAXED);

602 +#endif

603 + tinysleep();

604 + }

605 }

606

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

608 uint8_t *data = ptr;

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

495 - uintptr_t tagged_page = (uintptr_t)page; // Cast the pointer to an integer

496 - if (marked) tagged_page |= 1; // Set the LSB to 1 if 'marked' is true

497 - __atomic_store_n(page_ptr, tagged_page, __ATOMIC_RELEASE); // Atomically store the tagged pointer

610 + uintptr_t tagged_page = (uintptr_t)page;

611 + if (marked) tagged_page |= ARAL_TRAILER_MARKED;

612 + __atomic_store_n(page_ptr, tagged_page, __ATOMIC_RELEASE);

613 }

614

615 // --------------------------------------------------------------------------------------------------------------------

@@ -1010,18 +1125,44 @@ void aral_unmark_allocation(ARAL *ar, void *ptr) {

1125

1126 if(unlikely(!ptr)) return;

1127

1013 - // get the page pointer

1014 - bool marked;

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

1128 +#ifdef NETDATA_INTERNAL_CHECKS

1129 + aral_concurrency_race_pause(ar, ptr, ARAL_CONCURRENCY_RACE_UNMARK_BEFORE_CAS);

1130 +#endif

1131 +

1132 + uint8_t *data = ptr;

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

1134

1017 - internal_fatal(!marked, "This allocation does is not marked");

1135 + // Stage 1: claim the unmark transition.

1136 + // CAS the trailer from (page, MARKED) to (page, UNMARKING). On failure

1137 + // (slot was freed, already unmarked, another unmark won), bail without

1138 + // touching counters.

1139 + //

1140 + // Holding the UNMARKING state has two crucial effects:

1141 + // - aral_freez_internal observes UNMARKING and waits, so the slot's

1142 + // refcount contribution stays in place and the page cannot be

1143 + // destroyed under us.

1144 + // - freez never observes the slot as unmarked while marked_elements

1145 + // is still high, so the "marked > used" invariant is preserved.

1146 + uintptr_t initial = __atomic_load_n(page_ptr, __ATOMIC_ACQUIRE);

1147 + if((initial & ARAL_TRAILER_TAG_MASK) != ARAL_TRAILER_MARKED)

1148 + return;

1149 + uintptr_t desired = (initial & ~ARAL_TRAILER_TAG_MASK) | ARAL_TRAILER_UNMARKING;

1150 + if(!__atomic_compare_exchange_n(page_ptr, &initial, desired,

1151 + false, __ATOMIC_ACQ_REL, __ATOMIC_ACQUIRE))

1152 + return;

1153

1019 - if(marked)

1020 - aral_set_page_pointer_after_element___do_NOT_have_aral_lock(ar, page, ptr, false);

1154 +#ifdef NETDATA_INTERNAL_CHECKS

1155 + aral_concurrency_race_pause(ar, ptr, ARAL_CONCURRENCY_RACE_UNMARK_AFTER_CAS);

1156 +#endif

1157 +

1158 + // Stage 2: under page_lock, decrement counters and update page lists.

1159 + bool was_marked;

1160 + ARAL_PAGE *page = aral_decode_page_pointer_after_element___do_NOT_have_aral_lock(ar, ptr, initial, &was_marked);

1161 + (void)was_marked;

1162

1163 aral_page_lock(ar, page);

1023 - internal_fatal(marked && !page->page_lock.marked_elements, "Marked counter going negative.");

1024 - bool unmark = marked && --page->page_lock.marked_elements == 0 && page->page_lock.used_elements;

1164 + internal_fatal(!page->page_lock.marked_elements, "Marked counter going negative.");

1165 + bool unmark = (--page->page_lock.marked_elements == 0) && page->page_lock.used_elements;

1166

1167 if(unmark) {

1168 aral_lock(ar);

@@ -1041,6 +1182,12 @@ void aral_unmark_allocation(ARAL *ar, void *ptr) {

1182 aral_unlock(ar);

1183 }

1184

1185 + // Stage 3: publish the final UNMARKED state.

1186 + // Atomic store transitions the trailer from (page, UNMARKING) to (page, UNMARKED).

1187 + // Done under page_lock so any waiting freez observes a settled trailer

1188 + // only after our counter update is committed.

1189 + __atomic_store_n(page_ptr, (uintptr_t)page, __ATOMIC_RELEASE);

1190 +

1191 aral_page_unlock(ar, page);

1192 }

1193

@@ -1057,11 +1204,18 @@ void aral_freez_internal(ARAL *ar, void *ptr TRACE_ALLOCATIONS_FUNCTION_DEFINITI

1204

1205 if(unlikely(!ptr)) return;

1206

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.

1207 +#ifdef NETDATA_INTERNAL_CHECKS

1208 + aral_concurrency_race_pause(ar, ptr, ARAL_CONCURRENCY_RACE_FREEZ_BEFORE_CLAIM);

1209 +#endif

1210 +

1211 + // Atomically claim the trailer:

1212 + // - On a concurrent double-free or stale-free, the loser observes a

1213 + // NULL pointer and fatal()s here.

1214 + // - If aral_unmark_allocation has CAS'd the trailer to UNMARKING, we

1215 + // wait until it publishes the final UNMARKED state, so we never see

1216 + // the slot as unmarked while marked_elements is still high.

1217 bool marked;

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

1218 + ARAL_PAGE *page = aral_claim_page_pointer_after_element___wait_for_unmark(ar, ptr, &marked);

1219 if(unlikely(!page))

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

1221

@@ -1644,6 +1798,548 @@ static int aral_detect_acquire_to_page_lock_race(void) {

1798

1799 return errors;

1800 }

1801 +

1802 +// --------------------------------------------------------------------------------------------------------------------

1803 +// Concurrency tests for the unmark / freez state machine.

1804 +//

1805 +// Each scenario uses the aral_concurrency_race_hook to deterministically pause

1806 +// one operation at a known point so a second operation can race it. Tests 4-6

1807 +// drive both sides through the trailer transition concurrently and verify

1808 +// that page counters end consistent and no assertion fires; tests 1-3 are

1809 +// bug-independent sanity checks for the unmark / freez API contract.

1810 +

1811 +struct aral_concurrency_test_args {

1812 + ARAL *ar;

1813 + void *ptr;

1814 +};

1815 +

1816 +static void aral_concurrency_test_unmark_thread(void *arg) {

1817 + struct aral_concurrency_test_args *a = arg;

1818 + aral_unmark_allocation(a->ar, a->ptr);

1819 +}

1820 +

1821 +static void aral_concurrency_test_freez_thread(void *arg) {

1822 + struct aral_concurrency_test_args *a = arg;

1823 + aral_freez(a->ar, a->ptr);

1824 +}

1825 +

1826 +static ARAL_PAGE *aral_concurrency_test_decode(ARAL *ar, void *ptr, bool *marked) {

1827 + uint8_t *data = ptr;

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

1829 + uintptr_t tagged = __atomic_load_n(page_ptr, __ATOMIC_ACQUIRE);

1830 + return aral_decode_page_pointer_after_element___do_NOT_have_aral_lock(ar, ptr, tagged, marked);

1831 +}

1832 +

1833 +// Common setup for tests that need a marked guard and a marked test entry on

1834 +// the same page. Captures used/marked counters at setup time so each test can

1835 +// verify the expected delta after its scenario runs.

1836 +struct aral_concurrency_test_fixture {

1837 + ARAL *ar;

1838 + struct aral_unittest_entry *guard;

1839 + struct aral_unittest_entry *entry;

1840 + ARAL_PAGE *page;

1841 + uint32_t used0;

1842 + uint32_t marked0;

1843 +};

1844 +

1845 +// Initialize the fixture. Returns true on success. On failure (guard/entry

1846 +// landed on different pages), the aral and allocations are torn down and

1847 +// false is returned; the caller should bubble up an error.

1848 +static bool aral_concurrency_test_fixture_init(struct aral_concurrency_test_fixture *f, const char *name) {

1849 + f->ar = aral_create(name, sizeof(struct aral_unittest_entry),

1850 + 0, 0, NULL, name, NULL, false, false, false);

1851 +

1852 + // Guard is allocated marked so it lives on the same (marked) page as the

1853 + // test entry. Marked and unmarked allocations use separate page lists.

1854 + f->guard = aral_mallocz_marked(f->ar);

1855 + *f->guard = UNITTEST_ITEM;

1856 +

1857 + f->entry = aral_mallocz_marked(f->ar);

1858 + *f->entry = UNITTEST_ITEM;

1859 +

1860 + bool m = false;

1861 + f->page = aral_concurrency_test_decode(f->ar, f->entry, &m);

1862 + bool gm = false;

1863 + ARAL_PAGE *guard_page = aral_concurrency_test_decode(f->ar, f->guard, &gm);

1864 + if(guard_page != f->page) {

1865 + fprintf(stderr, " setup: guard and entry are not on the same page (guard=%p entry=%p)\n",

1866 + (void*)guard_page, (void*)f->page);

1867 + aral_freez(f->ar, f->entry);

1868 + aral_freez(f->ar, f->guard);

1869 + aral_destroy(f->ar);

1870 + return false;

1871 + }

1872 + f->used0 = f->page->page_lock.used_elements;

1873 + f->marked0 = f->page->page_lock.marked_elements;

1874 + return true;

1875 +}

1876 +

1877 +// Teardown when the test entry was already freed by the scenario.

1878 +// Frees the guard, destroys the aral, resets the concurrency hook.

1879 +static void aral_concurrency_test_fixture_teardown(struct aral_concurrency_test_fixture *f) {

1880 + aral_freez(f->ar, f->guard);

1881 + aral_destroy(f->ar);

1882 + aral_concurrency_race_hook_reset();

1883 +}

1884 +

1885 +// Teardown when the test entry is still allocated (e.g. failure path before

1886 +// the scenario could free it). Frees both entry and guard.

1887 +static void aral_concurrency_test_fixture_teardown_with_entry(struct aral_concurrency_test_fixture *f) {

1888 + aral_freez(f->ar, f->entry);

1889 + aral_freez(f->ar, f->guard);

1890 + aral_destroy(f->ar);

1891 + aral_concurrency_race_hook_reset();

1892 +}

1893 +

1894 +// Verify the page counters reflect exactly one freez of the test entry:

1895 +// used and marked each decremented by 1. Returns the error count to add.

1896 +static int aral_concurrency_test_check_counters_after_one_freez(struct aral_concurrency_test_fixture *f) {

1897 + int errors = 0;

1898 + if(f->page->page_lock.used_elements != f->used0 - 1) {

1899 + fprintf(stderr, " used_elements: %u -> %u (expected %u)\n",

1900 + f->used0, f->page->page_lock.used_elements, f->used0 - 1);

1901 + errors++;

1902 + }

1903 + if(f->page->page_lock.marked_elements != f->marked0 - 1) {

1904 + fprintf(stderr, " marked_elements: %u -> %u (expected %u)\n",

1905 + f->marked0, f->page->page_lock.marked_elements, f->marked0 - 1);

1906 + errors++;

1907 + }

1908 + return errors;

1909 +}

1910 +

1911 +// Test 1: clean unmark on a marked allocation. No concurrency.

1912 +// Expects: marked counter -1, used counter unchanged, slot trailer becomes UNMARKED.

1913 +static int aral_concurrency_test_clean_unmark(void) {

1914 + int errors = 0;

1915 + fprintf(stderr, " test 1: clean unmark on a marked allocation\n");

1916 +

1917 + ARAL *ar = aral_create("aral-conc-1", sizeof(struct aral_unittest_entry),

1918 + 0, 0, NULL, "aral-conc-1", NULL, false, false, false);

1919 +

1920 + struct aral_unittest_entry *entry = aral_mallocz_marked(ar);

1921 + *entry = UNITTEST_ITEM;

1922 +

1923 + bool m = false;

1924 + ARAL_PAGE *page = aral_concurrency_test_decode(ar, entry, &m);

1925 + if(!m) { fprintf(stderr, " setup: not marked\n"); errors++; }

1926 + uint32_t used0 = page->page_lock.used_elements;

1927 + uint32_t marked0 = page->page_lock.marked_elements;

1928 +

1929 + aral_unmark_allocation(ar, entry);

1930 +

1931 + bool m_after = true;

1932 + ARAL_PAGE *p_after = aral_concurrency_test_decode(ar, entry, &m_after);

1933 + if(p_after != page || m_after) {

1934 + fprintf(stderr, " trailer state wrong after unmark (page=%p marked=%d)\n", (void*)p_after, (int)m_after);

1935 + errors++;

1936 + }

1937 + if(page->page_lock.used_elements != used0) {

1938 + fprintf(stderr, " used_elements changed (%u -> %u)\n", used0, page->page_lock.used_elements);

1939 + errors++;

1940 + }

1941 + if(page->page_lock.marked_elements != marked0 - 1) {

1942 + fprintf(stderr, " marked_elements: %u -> %u (expected %u)\n",

1943 + marked0, page->page_lock.marked_elements, marked0 - 1);

1944 + errors++;

1945 + }

1946 +

1947 + aral_freez(ar, entry);

1948 + aral_destroy(ar);

1949 + return errors;

1950 +}

1951 +

1952 +// Test 2: unmark on an already-unmarked allocation. Should bail without changing counters.

1953 +static int aral_concurrency_test_unmark_on_unmarked(void) {

1954 + int errors = 0;

1955 + fprintf(stderr, " test 2: unmark on an already-unmarked allocation (should bail)\n");

1956 +

1957 + ARAL *ar = aral_create("aral-conc-2", sizeof(struct aral_unittest_entry),

1958 + 0, 0, NULL, "aral-conc-2", NULL, false, false, false);

1959 +

1960 + // allocate UNMARKED (regular mallocz)

1961 + struct aral_unittest_entry *entry = aral_mallocz(ar);

1962 + *entry = UNITTEST_ITEM;

1963 +

1964 + bool m = true;

1965 + ARAL_PAGE *page = aral_concurrency_test_decode(ar, entry, &m);

1966 + if(m) { fprintf(stderr, " setup: unexpectedly marked\n"); errors++; }

1967 + uint32_t used0 = page->page_lock.used_elements;

1968 + uint32_t marked0 = page->page_lock.marked_elements;

1969 +

1970 + aral_unmark_allocation(ar, entry);

1971 +

1972 + if(page->page_lock.used_elements != used0 || page->page_lock.marked_elements != marked0) {

1973 + fprintf(stderr, " counters changed after no-op unmark (used %u->%u, marked %u->%u)\n",

1974 + used0, page->page_lock.used_elements, marked0, page->page_lock.marked_elements);

1975 + errors++;

1976 + }

1977 +

1978 + aral_freez(ar, entry);

1979 + aral_destroy(ar);

1980 + return errors;

1981 +}

1982 +

1983 +// Test 3: clean freez on a marked allocation. No concurrency.

1984 +// Expects: used -1, marked -1, slot returns to free pool.

1985 +//

1986 +// A marked guard is kept alive on the same page so the page is not destroyed

1987 +// (or otherwise has its counters reset) by freezing the only allocation.

1988 +static int aral_concurrency_test_clean_freez_marked(void) {

1989 + int errors = 0;

1990 + fprintf(stderr, " test 3: clean freez of a marked allocation\n");

1991 +

1992 + struct aral_concurrency_test_fixture f;

1993 + if(!aral_concurrency_test_fixture_init(&f, "aral-conc-3"))

1994 + return 1;

1995 +

1996 + aral_freez(f.ar, f.entry);

1997 +

1998 + errors += aral_concurrency_test_check_counters_after_one_freez(&f);

1999 +

2000 + aral_concurrency_test_fixture_teardown(&f);

2001 + return errors;

2002 +}

2003 +

2004 +// Test 4: race - freez wins claim before unmark gets to its CAS.

2005 +// Pause unmark at entry, run freez to completion, release unmark.

2006 +// On master (no fix): unmark loads trailer=0, decode produces page=NULL,

2007 +// triggers "possible corruption or double free" internal_fatal under

2008 +// NETDATA_INTERNAL_CHECKS, OR aral_set_page_pointer with NULL page,

2009 +// then aral_page_lock(NULL) SEGV.

2010 +// On v2: unmark loads 0, the (initial & TAG_MASK) != MARKED check bails

2011 +// without touching counters or pointers.

2012 +// Either outcome is captured: master crashes, v2 passes counter checks.

2013 +//

2014 +// A guard allocation is kept alive on the same page so the page is never

2015 +// destroyed mid-test (refcount stays > 0), making it safe to read page

2016 +// counters after the racing freez completes.

2017 +static int aral_concurrency_test_freez_wins(void) {

2018 + int errors = 0;

2019 + fprintf(stderr, " test 4: race - freez wins claim before unmark CAS\n");

2020 +

2021 + struct aral_concurrency_test_fixture f;

2022 + if(!aral_concurrency_test_fixture_init(&f, "aral-conc-4"))

2023 + return 1;

2024 +

2025 + aral_concurrency_race_hook_arm(f.ar, f.entry, ARAL_CONCURRENCY_RACE_UNMARK_BEFORE_CAS);

2026 +

2027 + struct aral_concurrency_test_args ctx = { f.ar, f.entry };

2028 + ND_THREAD *unmark_thread = nd_thread_create("UNMARK", NETDATA_THREAD_OPTION_DONT_LOG,

2029 + aral_concurrency_test_unmark_thread, &ctx);

2030 + if(!unmark_thread) {

2031 + fprintf(stderr, " failed to create unmark thread\n");

2032 + aral_concurrency_test_fixture_teardown_with_entry(&f);

2033 + return errors + 1;

2034 + }

2035 +

2036 + if(!aral_unittest_wait_for_flag(&aral_concurrency_race_hook.waiting, 5 * USEC_PER_SEC)) {

2037 + fprintf(stderr, " unmark thread did not pause\n");

2038 + errors++;

2039 + }

2040 +

2041 + // The hook is armed for an UNMARK_* stage, so the freez we are about to

2042 + // run will not match (its only pause point is FREEZ_BEFORE_CLAIM). Just

2043 + // freez and let unmark resume on the release flag below.

2044 + aral_freez(f.ar, f.entry);

2045 +

2046 + __atomic_store_n(&aral_concurrency_race_hook.release, true, __ATOMIC_RELEASE);

2047 + nd_thread_join(unmark_thread);

2048 +

2049 + if(f.page->page_lock.used_elements != f.used0 - 1) {

2050 + fprintf(stderr, " used_elements: %u -> %u (expected %u)\n",

2051 + f.used0, f.page->page_lock.used_elements, f.used0 - 1);

2052 + errors++;

2053 + }

2054 + if(f.page->page_lock.marked_elements != f.marked0 - 1) {

2055 + fprintf(stderr, " marked_elements: %u -> %u (expected %u; %u would mean unmark double-decremented)\n",

2056 + f.marked0, f.page->page_lock.marked_elements, f.marked0 - 1, f.marked0 - 2);

2057 + errors++;

2058 + }

2059 +

2060 + aral_concurrency_test_fixture_teardown(&f);

2061 + return errors;

2062 +}

2063 +

2064 +// Tests 5 and 6: race - unmark wins the trailer transition, then freez runs.

2065 +// Pause unmark AFTER its trailer transition (UNMARKING on v2, UNMARKED on

2066 +// master) but BEFORE the page-lock counter update. Run freez concurrently:

2067 +// on v2 it should observe UNMARKING, restore, and spin until unmark publishes

2068 +// the final state; on master it observes UNMARKED, captures marked=false,

2069 +// then races on page_lock with unmark.

2070 +//

2071 +// Master under NETDATA_INTERNAL_CHECKS: if freez wins page_lock first, the

2072 +// "marked > used" assertion at aral_freez_internal fires (or the deletion

2073 +// path's "page has marked elements but not used ones" fires). This is the

2074 +// exact race the v2 protocol closes.

2075 +//

2076 +// v2: counters end consistent: used -1, marked -1.

2077 +//

2078 +// Test 5 sleeps 10ms before releasing unmark to let freez settle into its

2079 +// cold-path spin; test 6 releases immediately to also catch regressions in

2080 +// the very first iteration of the cold path.

2081 +static int aral_concurrency_test_unmark_wins_impl(const char *aral_name, usec_t grace_us) {

2082 + int errors = 0;

2083 +

2084 + struct aral_concurrency_test_fixture f;

2085 + if(!aral_concurrency_test_fixture_init(&f, aral_name))

2086 + return 1;

2087 +

2088 + aral_concurrency_race_hook_arm(f.ar, f.entry, ARAL_CONCURRENCY_RACE_UNMARK_AFTER_CAS);

2089 +

2090 + struct aral_concurrency_test_args ctx = { f.ar, f.entry };

2091 + ND_THREAD *unmark_thread = nd_thread_create("UNMARK", NETDATA_THREAD_OPTION_DONT_LOG,

2092 + aral_concurrency_test_unmark_thread, &ctx);

2093 + if(!unmark_thread) {

2094 + fprintf(stderr, " failed to create unmark thread\n");

2095 + aral_concurrency_test_fixture_teardown_with_entry(&f);

2096 + return errors + 1;

2097 + }

2098 +

2099 + if(!aral_unittest_wait_for_flag(&aral_concurrency_race_hook.waiting, 5 * USEC_PER_SEC)) {

2100 + fprintf(stderr, " unmark thread did not pause after trailer transition\n");

2101 + errors++;

2102 + __atomic_store_n(&aral_concurrency_race_hook.release, true, __ATOMIC_RELEASE);

2103 + nd_thread_join(unmark_thread);

2104 + aral_concurrency_test_fixture_teardown_with_entry(&f);

2105 + return errors;

2106 + }

2107 +

2108 + // Hook is armed for UNMARK_AFTER_CAS - freez's FREEZ_BEFORE_CLAIM pause

2109 + // point will not match, so the freez thread proceeds without pausing.

2110 + ND_THREAD *freez_thread = nd_thread_create("FREEZ", NETDATA_THREAD_OPTION_DONT_LOG,

2111 + aral_concurrency_test_freez_thread, &ctx);

2112 + if(!freez_thread) {

2113 + fprintf(stderr, " failed to create freez thread\n");

2114 + __atomic_store_n(&aral_concurrency_race_hook.release, true, __ATOMIC_RELEASE);

2115 + nd_thread_join(unmark_thread);

2116 + // unmark completed - it transitioned the slot to UNMARKED but did not

2117 + // free it. We must free entry too.

2118 + aral_concurrency_test_fixture_teardown_with_entry(&f);

2119 + return errors + 1;

2120 + }

2121 +

2122 + if(grace_us > 0)

2123 + sleep_usec(grace_us);

2124 +

2125 + __atomic_store_n(&aral_concurrency_race_hook.release, true, __ATOMIC_RELEASE);

2126 + nd_thread_join(unmark_thread);

2127 + nd_thread_join(freez_thread);

2128 +

2129 + errors += aral_concurrency_test_check_counters_after_one_freez(&f);

2130 +

2131 + aral_concurrency_test_fixture_teardown(&f);

2132 + return errors;

2133 +}

2134 +

2135 +static int aral_concurrency_test_unmark_wins_transition(void) {

2136 + fprintf(stderr, " test 5: race - unmark transitions trailer, freez races counter update\n");

2137 + return aral_concurrency_test_unmark_wins_impl("aral-conc-5", 10 * USEC_PER_MS);

2138 +}

2139 +

2140 +static int aral_concurrency_test_unmark_wins_no_grace(void) {

2141 + fprintf(stderr, " test 6: race - same as test 5, no grace period before release\n");

2142 + return aral_concurrency_test_unmark_wins_impl("aral-conc-6", 0);

2143 +}

2144 +

2145 +// Test 7: unmark of the last marked element on a page triggers a list move

2146 +// from the marked-pages list to the unmarked-pages list. This is the

2147 +// "if(unmark)" branch of aral_unmark_allocation that takes aral_lock and

2148 +// moves the page between linked lists.

2149 +static int aral_concurrency_test_unmark_last_marked_on_page(void) {

2150 + int errors = 0;

2151 + fprintf(stderr, " test 7: unmark of last marked element triggers list move\n");

2152 +

2153 + ARAL *ar = aral_create("aral-conc-7", sizeof(struct aral_unittest_entry),

2154 + 0, 0, NULL, "aral-conc-7", NULL, false, false, false);

2155 +

2156 + // Single marked allocation. After unmarking, marked_elements drops to 0

2157 + // while used_elements stays at 1 (unmarking does not free the slot), which

2158 + // triggers the unmark branch in aral_unmark_allocation that takes

2159 + // aral_lock and moves the page from the marked-pages list to the

2160 + // unmarked-pages list.

2161 + struct aral_unittest_entry *m1 = aral_mallocz_marked(ar);

2162 + *m1 = UNITTEST_ITEM;

2163 +

2164 + bool m = false;

2165 + ARAL_PAGE *marked_page = aral_concurrency_test_decode(ar, m1, &m);

2166 + if(!m) { fprintf(stderr, " setup: not marked\n"); errors++; }

2167 + if(!marked_page->aral_lock.marked) {

2168 + fprintf(stderr, " setup: page not on marked list before unmark\n");

2169 + errors++;

2170 + }

2171 +

2172 + aral_unmark_allocation(ar, m1);

2173 +

2174 + if(marked_page->page_lock.marked_elements != 0) {

2175 + fprintf(stderr, " marked_elements not 0 after unmark of last marked: %u\n",

2176 + marked_page->page_lock.marked_elements);

2177 + errors++;

2178 + }

2179 + if(marked_page->aral_lock.marked) {

2180 + fprintf(stderr, " page still on marked list after unmarking last marked\n");

2181 + errors++;

2182 + }

2183 +

2184 + aral_freez(ar, m1);

2185 + aral_destroy(ar);

2186 + return errors;

2187 +}

2188 +

2189 +// Test 8: coordinated per-pointer race stress.

2190 +// For every pointer in the pool, deterministically force the racing window:

2191 +// 1. Arm the hook to pause unmark at the UNMARKING transition.

2192 +// 2. Spawn an unmark thread - it CAS's to UNMARKING and pauses at the hook.

2193 +// 3. Spawn a freez thread on the same pointer - it observes UNMARKING and

2194 +// enters the cold path (restore + retry) of the claim helper.

2195 +// 4. Release unmark - it finishes its counter update and publishes UNMARKED.

2196 +// 5. Freez's retry exchange picks up UNMARKED, claims, decrements only used.

2197 +// 6. Both threads complete; the slot is fully freed.

2198 +//

2199 +// This exercises every interesting transition for every pointer:

2200 +// - unmark wins the trailer transition

2201 +// - freez observes UNMARKING (cold path of the claim helper)

2202 +// - the published UNMARKED value lets freez proceed

2203 +// Plus, because the pool spans multiple pages, the freezes near the end of

2204 +// each page exercise the "last marked element triggers list move" branch and

2205 +// the page deletion path.

2206 +//

2207 +// Final state is verified via aral_used_bytes(): must be 0 after every slot

2208 +// is freed.

2209 +static int aral_concurrency_test_stress(void) {

2210 + int errors = 0;

2211 + const size_t pool_size = 256; // large enough to span multiple pages

2212 + fprintf(stderr, " test 8: coordinated race stress - %zu pointers, deterministic UNMARKING for each\n", pool_size);

2213 +

2214 + ARAL *ar = aral_create("aral-conc-stress", sizeof(struct aral_unittest_entry),

2215 + 0, 0, NULL, "aral-conc-stress", NULL, false, false, false);

2216 +

2217 + struct aral_unittest_entry **pool = callocz(pool_size, sizeof(*pool));

2218 + for(size_t i = 0; i < pool_size; i++) {

2219 + pool[i] = aral_mallocz_marked(ar);

2220 + *pool[i] = UNITTEST_ITEM;

2221 + }

2222 +

2223 + size_t cold_path_hits = 0;

2224 + size_t i;

2225 + for(i = 0; i < pool_size; i++) {

2226 + // Arm the hook to pause unmark just after its CAS to UNMARKING.

2227 + aral_concurrency_race_hook_arm(ar, pool[i], ARAL_CONCURRENCY_RACE_UNMARK_AFTER_CAS);

2228 +

2229 + struct aral_concurrency_test_args ctx = { ar, pool[i] };

2230 +

2231 + ND_THREAD *unmark_thread = nd_thread_create("UNMARK", NETDATA_THREAD_OPTION_DONT_LOG,

2232 + aral_concurrency_test_unmark_thread, &ctx);

2233 + if(!unmark_thread) {

2234 + fprintf(stderr, " iter %zu: failed to create unmark thread\n", i);

2235 + errors++;

2236 + break;

2237 + }

2238 +

2239 + if(!aral_unittest_wait_for_flag(&aral_concurrency_race_hook.waiting, 5 * USEC_PER_SEC)) {

2240 + fprintf(stderr, " iter %zu: unmark did not reach pause\n", i);

2241 + errors++;

2242 + __atomic_store_n(&aral_concurrency_race_hook.release, true, __ATOMIC_RELEASE);

2243 + nd_thread_join(unmark_thread);

2244 + break;

2245 + }

2246 +

2247 + // Hook is armed for UNMARK_AFTER_CAS - freez's FREEZ_BEFORE_CLAIM

2248 + // pause point will not match, so the freez we are about to spawn

2249 + // proceeds without pausing.

2250 +

2251 + // Snapshot the UNMARKING-cold-path counter before spawning freez.

2252 + size_t cold_before = __atomic_load_n(&aral_freez_unmarking_observed_count, __ATOMIC_ACQUIRE);

2253 +

2254 + ND_THREAD *freez_thread = nd_thread_create("FREEZ", NETDATA_THREAD_OPTION_DONT_LOG,

2255 + aral_concurrency_test_freez_thread, &ctx);

2256 + if(!freez_thread) {

2257 + fprintf(stderr, " iter %zu: failed to create freez thread\n", i);

2258 + errors++;

2259 + __atomic_store_n(&aral_concurrency_race_hook.release, true, __ATOMIC_RELEASE);

2260 + nd_thread_join(unmark_thread);

2261 + break;

2262 + }

2263 +

2264 + // Wait until freez actually enters the UNMARKING cold path, i.e. its

2265 + // exchange returned a value with the UNMARKING bit set. Without this

2266 + // synchronization the test could release unmark before freez has

2267 + // even reached the exchange, never exercising the cold path we are

2268 + // here to validate.

2269 + usec_t deadline = now_monotonic_usec() + 5 * USEC_PER_SEC;

2270 + while(__atomic_load_n(&aral_freez_unmarking_observed_count, __ATOMIC_ACQUIRE) == cold_before) {

2271 + if(now_monotonic_usec() > deadline) {

2272 + fprintf(stderr, " iter %zu: freez did not observe UNMARKING within timeout\n", i);

2273 + errors++;

2274 + break;

2275 + }

2276 + tinysleep();

2277 + }

2278 + if(__atomic_load_n(&aral_freez_unmarking_observed_count, __ATOMIC_ACQUIRE) > cold_before)

2279 + cold_path_hits++;

2280 +

2281 + // Now release unmark; freez (still spinning in its cold path) will

2282 + // observe the published UNMARKED and proceed.

2283 + __atomic_store_n(&aral_concurrency_race_hook.release, true, __ATOMIC_RELEASE);

2284 +

2285 + nd_thread_join(unmark_thread);

2286 + nd_thread_join(freez_thread);

2287 +

2288 + // Reset the hook for the next iteration.

2289 + aral_concurrency_race_hook_reset();

2290 + }

2291 +

2292 + // Defensive reset: any early break above could have left the hook armed.

2293 + aral_concurrency_race_hook_reset();

2294 +

2295 + // Free pool entries that were not freed by a successful loop iteration.

2296 + // On normal completion i == pool_size and this loop is empty. On early

2297 + // break, pool[i] may be in any allocated state (MARKED or UNMARKED) but

2298 + // is always still allocated; aral_freez handles both.

2299 + for(size_t j = i; j < pool_size; j++)

2300 + aral_freez(ar, pool[j]);

2301 +

2302 + if(cold_path_hits != pool_size) {

2303 + fprintf(stderr, " cold path was exercised %zu/%zu times (expected all)\n",

2304 + cold_path_hits, pool_size);

2305 + errors++;

2306 + }

2307 +

2308 + if(aral_used_bytes(ar) != 0) {

2309 + fprintf(stderr, " aral has %zu used bytes after stress test (expected 0)\n",

2310 + aral_used_bytes(ar));

2311 + errors++;

2312 + }

2313 +

2314 + freez(pool);

2315 + aral_destroy(ar);

2316 + return errors;

2317 +}

2318 +

2319 +int aral_unittest_concurrency(void) {

2320 +#if defined(FSANITIZE_ADDRESS)

2321 + // Under address sanitizer ARAL is bypassed entirely: mallocz/callocz/

2322 + // freez delegate straight to glibc and aral_unmark_allocation() is a

2323 + // no-op. There is no trailer protocol to test, so skip cleanly.

2324 + fprintf(stderr, "ARAL concurrency tests: SKIPPED (ARAL is disabled under FSANITIZE_ADDRESS)\n");

2325 + return 0;

2326 +#else

2327 + fprintf(stderr, "Running ARAL concurrency tests (unmark/freez state machine)...\n");

2328 + int errors = 0;

2329 + errors += aral_concurrency_test_clean_unmark();

2330 + errors += aral_concurrency_test_unmark_on_unmarked();

2331 + errors += aral_concurrency_test_clean_freez_marked();

2332 + errors += aral_concurrency_test_freez_wins();

2333 + errors += aral_concurrency_test_unmark_wins_transition();

2334 + errors += aral_concurrency_test_unmark_wins_no_grace();

2335 + errors += aral_concurrency_test_unmark_last_marked_on_page();

2336 + errors += aral_concurrency_test_stress();

2337 + fprintf(stderr, "ARAL concurrency tests: %s (%d errors)\n",

2338 + errors ? "FAILED" : "PASSED", errors);

2339 + return errors;

2340 +#endif

2341 +}

2342 +

2343 #endif

2344

2345 static void aral_test_thread(void *ptr) {

src/libnetdata/aral/aral.h

+9 -2

@@ -124,12 +124,19 @@ void *aral_mallocz_internal(ARAL *ar, bool marked);

124 void aral_freez_internal(ARAL *ar, void *ptr);

125 void aral_destroy_internal(ARAL *ar);

126

127 -void aral_unmark_allocation(ARAL *ar, void *ptr);

127 +#endif // NETDATA_TRACE_ALLOCATIONS

128

129 // --------------------------------------------------------------------------------------------------------------------

130 +// Declarations that do not depend on the NETDATA_TRACE_ALLOCATIONS macro

131 +// shape, kept outside the conditional so callers in any compilation unit

132 +// can see them in both build modes.

133 +

134 +void aral_unmark_allocation(ARAL *ar, void *ptr);

135

136 int aral_unittest(size_t elements);

137

133 -#endif // NETDATA_TRACE_ALLOCATIONS

138 +#ifdef NETDATA_INTERNAL_CHECKS

139 +int aral_unittest_concurrency(void);

140 +#endif

141

142 #endif // ARAL_H