feat(pool): opt-in debug hardening — freed-block poisoning, a guard word, and free-list safe-linking#114
Merged
Merged
Conversation
…linking Harden the intrusive free list against the classic use-after-free / pointer-corruption primitives an in-band next-link exposes (ADR-0009 §1), behind a single compile-time knob `PBR_MEMORY_POOL_HARDENING` (OFF by default; a `harden` CMake preset turns it on). Three layered protections: - Freed-block poisoning (0xDE), verified on the next allocation — use-after-free detection. - A per-slot trailing guard word: neither-constant on free is a buffer overflow past block_size, still-freed is a double-free (closing the ADR-0012 gap). The guard lives in *added* slot stride, so the user-visible block_size and the ADR-0009 alignment guarantee are unchanged. - glibc-style free-list safe-linking (ptr XOR (slot_addr >> 12)): a leaked/overwritten next-link is neither directly usable nor silently followed; corruption surfaces as an alignment fault on reveal. On detection a swappable HardeningViolationHandler fires; the default prints a diagnostic and abort()s (the ADR-0012 defined-loud-failure stance), and tests install a recording handler to assert detection without terminating the process. The knob is fully compiled out when off, so the default build is byte-for-byte and cycle-for-cycle unchanged (read_next/write_next/ reveal_next/slot_stride inline to the exact prior load/store). Works with fixed and dynamic pools across all three thread-safety policies; composes under the InstrumentedPool decorator. Purely additive and ABI-compatible (SemVer MINOR, a v1.2.0 candidate); a hardened build is deliberately not layout-compatible with a non-hardened one. A `harden` CI matrix cell builds and runs the detection tests on each Tier-1 platform (the memory-safety net where ASan is unavailable, e.g. MSVC). Decided in ADR-0043; ROADMAP item 9.2. Closes #109. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Under the LOCKFREE thread-safety policy the pop path uses reveal_next (the integrity-check-free reveal for its speculative read), so the shared read_next helper has no caller unless the diagnostic surface is also compiled in. A LOCKFREE + diagnostics-off build (the `bench` preset policy-smoke cell) therefore tripped -Werror=unused-function on GCC. MSVC's equivalent (C4505) is off by default, so a local MSVC build did not surface it. [[maybe_unused]] is the portable suppression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two findings the CI diff gate surfaces on the files this branch touches (they had lain latent on master since the tidy gate last ran on memory_pool.cpp — the intervening PR was header-only): - initialize_free_list's two size_t parameters trip bugprone-easily-swappable-parameters. Suppress it as the create / factory entry points already do (they mirror the same frozen argument order), rather than diverging into a config struct. - The FreeListView walk test crossed the cognitive-complexity threshold (41 > 40) once the stride-derivation and its assertions were added. Extract the stride derivation into a derive_stride helper and move the stride/alignment assertions into their own focused TEST_CASE, returning the walk case to its prior complexity while keeping full coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
danielPoloWork
marked this pull request as ready for review
July 9, 2026 18:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements ROADMAP item 9.2 — opt-in debug hardening (issue #109), decided in ADR-0043. The pool's intrusive free list is a classic use-after-free / pointer-corruption primitive (a free block stores the next-link inside its own first
sizeof(void*)bytes — ADR-0009 §1). This PR adds a self-contained, opt-in hardening mode behind a single compile-time knob,PBR_MEMORY_POOL_HARDENING(a CMakeoption, OFF by default; ahardenpreset turns it on), that catches those failure modes deterministically everywhere the library builds — including MSVC, where ASan is unavailable and cannot see inside the pool's own region.What it adds
0xDE) over[sizeof(void*), block_size), verified intact on the next allocation → use-after-free.GUARD_ALLOCATED/GUARD_FREED): neither-constant on free → buffer overflow pastblock_size; still-freed → double-free (closing the ADR-0012 gap). The guard lives in added slot stride (stride = roundup(block_size + sizeof(guard), alignof(max_align_t))), so the user-visibleblock_sizeand the ADR-0009 alignment guarantee are unchanged.ptr XOR (slot_addr >> 12), glibc'sPROTECT_PTR/REVEAL_PTR); a leaked/overwritten link is neither directly usable nor silently followed, and corruption surfaces as an alignment fault on reveal.pool_hardening.hpp) — the default prints a diagnostic andabort()s (ADR-0012 defined-loud-failure); tests install a recording handler to assert detection without terminating the process.Zero cost when off
The entire mechanism is preprocessed out when the knob is off:
slot_stride/read_next/write_next/reveal_nextinline to the exact*static_cast<void**>(slot)load/store the pool has always used. The default build (and its benchmark/overhead numbers) is byte-for-byte and cycle-for-cycle unchanged — verified by the existingzero_overheadcheck. Purely additive and ABI-compatible → SemVer MINOR (av1.2.0candidate). A hardened build is deliberately not layout-compatible with a non-hardened one (never mix configurations).Scope
Works with fixed and dynamic pools across all three thread-safety policies (
NONE/MUTEX/LOCKFREE); the lock-free pop uses an integrity-check-free reveal on its speculative read and defers integrity to the owned-slot guard check after the CAS. Composes under theInstrumentedPooldecorator.Validation
ctestgreen in both the default (debug) andhardenconfigurations locally (MSVC 19.51); thepool_hardeningsuite proves no-false-positive cycles plus deterministic detection of use-after-free, overflow, and double-free.clang-formatclean;clang-tidyclean on the new code in both configurations.consistency_lint.pyOK (the spec's translation rows are alreadystale, so this edit adds no i18n debt — see ADR-0043).hardenCI matrix cell builds and runs the detection tests on each Tier-1 platform (Ubuntu gcc/clang, Windows MSVC, macOS apple-clang).Docs kept in sync
ADR-0043 (+ index row), spec §4.1/§5.3/§6.4/§7,
SECURITY.md,ROADMAP.md(9.2 → done),CHANGELOG.md[Unreleased], and the DoxyfilePREDEFINED. Per ADR-0043, theREADME.mdsecurity-section refresh + Milestone-9 row and thezh-Hans/jare-sync are deferred to thev1.2.0release PR to keep this feature PR off the translated docs surface.Closes #109.
🤖 Generated with Claude Code