Fix aral race condition by stelfrag · Pull Request #22367 · netdata/netdata · GitHub

Summary

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) under NETDATA_INTERNAL_CHECKS and reachable via -W aralconcurrency:

Cases:

1-3 clean unmark / unmark-on-unmarked / clean freez (with marked guard on the same page)
4-6 forced 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

Summary by cubic

Serialize ARAL unmark/freez with a new UNMARKING trailer state to close the race that caused counter skew and page‑lifetime UAFs. Free path stays fast; adds deterministic concurrency tests and a CLI flag to run them.

Bug Fixes
- Introduced ARAL_TRAILER_UNMARKING and ARAL_TRAILER_TAG_MASK; decode masks both; guarded by _Static_assert for alignment.
- aral_unmark_allocation() CASes (page|MARKED) -> (page|UNMARKING) before taking page_lock, decrements counters, then stores (page) under lock; early‑exit if freed, already unmarked, or CAS loses.
- aral_freez_internal() keeps the hot path as a single atomic exchange; if UNMARKING is seen, restores the tag (CAS if still zero), yields briefly, and retries; counts cold‑path hits via aral_freez_unmarking_observed_count under NETDATA_INTERNAL_CHECKS.
New Features
- Added aral_unittest_concurrency() (8 scenarios) with race hooks at UNMARK_BEFORE_CAS, UNMARK_AFTER_CAS, and FREEZ_BEFORE_CLAIM; includes a multi‑page stress that verifies the cold path; skipped under FSANITIZE_ADDRESS.
- Exposed via -W aralconcurrency; gated by NETDATA_INTERNAL_CHECKS with a clear stderr message when disabled; declarations updated in aral.h.

Written for commit 0e7d02e. Summary will update on new commits.

…nter 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

No issues found across 3 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

Architecture diagram

sequenceDiagram
    participant C as Caller
    participant AU as aral_unmark_allocation
    participant AF as aral_freez_internal
    participant T as Slot Trailer (Atomic Word)
    participant P as ARAL Page (Counters & Lock)

    Note over AU, AF: Concurrent access to the same allocation slot

    rect rgb(240, 245, 255)
        Note right of AU: UNMARKING Phase
        AU->>T: NEW: atomic_compare_exchange(MARKED -> UNMARKING)
        Note over T: State: UNMARKING (Stays in place to keep Page alive)
    end

    rect rgb(255, 245, 240)
        Note right of AF: FREEZ (Claim) Phase
        AF->>T: CHANGED: atomic_exchange(0)
        T-->>AF: Returns UNMARKING
        
        alt UNMARKING Observed (Cold Path)
            AF->>T: NEW: restore UNMARKING via CAS(0, UNMARKING)
            AF->>AF: tinysleep() / yield
        end
    end

    rect rgb(240, 245, 255)
        Note right of AU: Counter Update Phase
        AU->>P: aral_page_lock()
        AU->>P: CHANGED: decrement page->marked_elements
        
        alt marked_elements == 0
            AU->>P: Move page to appropriate ARAL list
        end
        
        AU->>T: NEW: atomic_store(UNMARKED)
        AU->>P: aral_page_unlock()
    end

    rect rgb(255, 245, 240)
        Note right of AF: Retry Phase
        AF->>T: atomic_exchange(0)
        T-->>AF: Returns UNMARKED
        AF->>P: aral_page_lock()
        AF->>P: decrement page->used_elements
        Note over P: Counter Consistency: marked_elements <= used_elements
        AF->>P: aral_page_unlock()
        AF-->>C: Slot freed successfully
    end

Pull request overview

This PR hardens ARAL’s unmark/freez concurrency by introducing a third trailer state (UNMARKING) to prevent counter invariants from being observed in an inconsistent state and to avoid page lifetime/UAF hazards during the trailer transition. It also adds a dedicated concurrency-focused unit test suite and exposes it via a new daemon CLI test option.

Changes:

Add a new trailer tag bit/state (UNMARKING) and update trailer encode/decode + freez claim logic to wait/retry when UNMARKING is observed.
Update aral_unmark_allocation() to CAS to UNMARKING before taking page_lock, then publish final UNMARKED trailer state after counters are updated.
Add aral_unittest_concurrency() (8 scenarios) and wire it into aral_unittest() and -W aralconcurrency.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/libnetdata/aral/aral.h	Declares the new aral_unittest_concurrency() entry point under NETDATA_INTERNAL_CHECKS.
src/libnetdata/aral/aral.c	Implements the UNMARKING trailer state machine, updates unmark/freez logic, and adds the new concurrency unit tests + deterministic race hook.
src/daemon/main.c	Adds -W aralconcurrency to run the new ARAL concurrency unit tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Fixes a race between aral_unmark_allocation() and aral_freez_internal() by introducing an UNMARKING trailer state, ensuring page counters and page lifetime remain consistent during the unmark/freez transition. Adds deterministic concurrency tests for the new unmark/freez state machine and wires them into the daemon unittest CLI option.

Changes:

Add ARAL_TRAILER_UNMARKING trailer state and update trailer encode/decode and free/unmark logic to wait out the transition safely.
Add aral_unittest_concurrency() covering multiple deterministic unmark/freez race scenarios (including a stress scenario).
Add -W aralconcurrency daemon option to run the concurrency tests under NETDATA_INTERNAL_CHECKS.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
src/libnetdata/aral/aral.h	Exposes the new concurrency unittest API (guarded by NETDATA_INTERNAL_CHECKS).
src/libnetdata/aral/aral.c	Implements the UNMARKING trailer protocol, updates free/unmark interactions, and adds deterministic concurrency tests.
src/daemon/main.c	Adds CLI hook (-W aralconcurrency) to run the new ARAL concurrency tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Fixes a concurrency bug in ARAL’s per-slot trailer protocol by introducing an UNMARKING intermediate trailer state to serialize aral_unmark_allocation() vs aral_freez_internal(), preventing counter inconsistencies and page lifetime UAFs; adds deterministic concurrency tests and a CLI entry point to run them.

Changes:

Introduces ARAL_TRAILER_UNMARKING and updates trailer encode/decode masking to support 2 tag bits with a compile-time alignment guard.
Updates aral_unmark_allocation() to CAS MARKED -> UNMARKING, then update counters under page_lock, then publish final UNMARKED.
Updates aral_freez_internal() to wait/retry if it observes UNMARKING, and adds aral_unittest_concurrency() plus -W aralconcurrency wiring.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/libnetdata/aral/aral.h	Exposes aral_unittest_concurrency() under NETDATA_INTERNAL_CHECKS and adjusts preprocessor structure around unittest entry points.
src/libnetdata/aral/aral.c	Implements the UNMARKING trailer state machine, updates unmark/freez synchronization, and adds deterministic concurrency unit tests.
src/daemon/main.c	Adds -W aralconcurrency CLI option to run the new ARAL concurrency tests (guarded by NETDATA_INTERNAL_CHECKS).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cubic-dev-ai review this PR

@cubic-dev-ai review this PR

@stelfrag I have started the AI code review. It will take a few minutes to complete.

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Tested during few hours on Debian 13, and everything worked as expected. LGTM!

* 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> (cherry picked from commit e8caf80)

github-actions Bot added the area/daemon label May 1, 2026

stelfrag marked this pull request as ready for review May 1, 2026 16:02

stelfrag requested review from thiagoftsm and vkalintiris as code owners May 1, 2026 16:02

Copilot AI review requested due to automatic review settings May 1, 2026 16:02

stelfrag marked this pull request as draft May 1, 2026 16:02

Copilot started reviewing on behalf of stelfrag May 1, 2026 16:02 View session

cubic-dev-ai Bot reviewed May 1, 2026

View reviewed changes

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/libnetdata/aral/aral.c Outdated

Address review comments

0abdf3c

stelfrag requested a review from Copilot May 1, 2026 18:27

Copilot started reviewing on behalf of stelfrag May 1, 2026 18:27 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/libnetdata/aral/aral.h

Comment thread src/libnetdata/aral/aral.c Outdated

Comment thread src/libnetdata/aral/aral.c Outdated

Comment thread src/libnetdata/aral/aral.c Outdated

Comment thread src/libnetdata/aral/aral.c Outdated

stelfrag added 2 commits May 1, 2026 21:57

Address review comments (2)

39035df

Address review comments (3)

2575070

stelfrag requested a review from Copilot May 1, 2026 19:18

Copilot started reviewing on behalf of stelfrag May 1, 2026 19:19 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/libnetdata/aral/aral.h Outdated

Comment thread src/libnetdata/aral/aral.c

Address review comments (4)

c21c3b6

stelfrag requested a review from Copilot May 1, 2026 19:46

Copilot started reviewing on behalf of stelfrag May 1, 2026 19:47 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/libnetdata/aral/aral.c Outdated

Update src/libnetdata/aral/aral.c

f00c67c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

stelfrag requested a review from Copilot May 1, 2026 19:56

Copilot started reviewing on behalf of stelfrag May 1, 2026 19:56 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/libnetdata/aral/aral.c Outdated

Comment thread src/libnetdata/aral/aral.c Outdated

Address review comments (5)

a5d54d4

stelfrag requested a review from Copilot May 1, 2026 20:21

Copilot started reviewing on behalf of stelfrag May 1, 2026 20:22 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/daemon/main.c

Address review comments (6)

b9a8bdb

stelfrag requested a review from Copilot May 2, 2026 07:51

Copilot started reviewing on behalf of stelfrag May 2, 2026 07:52 View session

Copilot AI reviewed May 2, 2026

View reviewed changes

Comment thread src/libnetdata/aral/aral.c Outdated

Comment thread src/libnetdata/aral/aral.c Outdated

Address review comments (7)

0e7d02e

stelfrag requested a review from Copilot May 2, 2026 08:03

Copilot started reviewing on behalf of stelfrag May 2, 2026 08:04 View session

Copilot AI reviewed May 2, 2026

View reviewed changes

stelfrag marked this pull request as ready for review May 2, 2026 08:51

thiagoftsm approved these changes May 2, 2026

View reviewed changes

stelfrag merged commit e8caf80 into netdata:master May 3, 2026
162 checks passed

stelfrag deleted the fix_aral_unmark_v2 branch May 3, 2026 15:16

stelfrag mentioned this pull request Jun 22, 2026

Patch release 2.10.4 #22825

Merged

Fix aral race condition by stelfrag · Pull Request #22367 · netdata/netdata · GitHub

Uh oh!

Fix aral race condition - #22367

Fix aral race condition#22367stelfrag merged 9 commits intonetdata:masternetdata/netdata:masterfrom stelfrag:fix_aral_unmark_v2stelfrag/netdata:fix_aral_unmark_v2Copy head branch name to clipboard

Conversation

stelfrag commented May 1, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by cubic

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

stelfrag commented May 2, 2026

Uh oh!

cubic-dev-ai Bot commented May 2, 2026

Uh oh!

sonarqubecloud Bot commented May 2, 2026

Quality Gate passed

Uh oh!

thiagoftsm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Fix aral race condition#22367
stelfrag merged 9 commits into
netdata:masterfrom
stelfrag:fix_aral_unmark_v2

stelfrag commented May 1, 2026 •
edited by cubic-dev-ai Bot
Loading