Skip to content
This repository was archived by the owner on Aug 20, 2026. It is now read-only.
This repository was archived by the owner on Aug 20, 2026. It is now read-only.

reg_field/reg_flow: out-of-bounds weight-map read under Zero boundary #39

Description

@balbasty

Summary

Under Zero boundary (and NoCheck), bound::utils::index() is the identity — it does not clamp/reflect an out-of-range neighbour index back into [0, size). Several RLS/JRLS weight-map reads assume index() always returns something dereferenceable and read the weight buffer via the raw, unchecked bound::cget(wgt, o) (2-arg overload — an unconditional ptr[offset]) instead of the sign-aware 3-arg overload (bound::cget(wgt, o, sign), which returns 0 when sign == 0, i.e. "no such neighbour"). Under Zero boundary this dereferences past the weight buffer.

Confirmed via Valgrind for the analogous flow-module bug (already fixed in #37): genuine heap-buffer-overflow, not just latent UB — the garbage value fed directly into the convolution broke self-adjointness and produced sporadic, heap-layout-dependent wrong results.

Where this still applies

  • regularisers/field/2d.h and 3d.h: matvec_bending_rls/_jrls, diag_bending_rls/_jrls (and likely matvec_membrane_rls/_jrls, matvec_absolute_rls/_jrls — same wget pattern, not yet audited one-by-one). For membrane/absolute this is latent UB that (so far) hasn't produced observable symmetry-test failures, likely because the garbage lands somewhere it gets discounted; for bending it is why the Zero-boundary case is currently excluded from the symmetry test oracle (see reg_field: bending RLS/JRLS (matvec_bending_rls/_jrls) self-adjointness bug #34, now closed, and fastfields-cpu-lib's test_reg_field.cpp bending-order loop, which only exercises DCT2/DST2/DFT).

Confirmed scope (re-audited, one-by-one, against current main)

Every wget in regularisers/field/{1d,2d,3d}.h still does the raw, unguarded 2-arg cget — this is not limited to bending: matvec_membrane_rls, diag_membrane_rls, matvec_membrane_jrls, diag_membrane_jrls, matvec_bending_rls, diag_bending_rls, matvec_bending_jrls, diag_bending_jrls (8 functions across 1D/2D/3D) all have the sign flags computed in scope and unused by wget.

Additionally, the fix in #37 only covered the flow module's matvec_membrane_jrls/diag_membrane_jrls. matvec_bending_jrls/diag_bending_jrls in regularisers/flow/2d.h and flow/3d.h have the identical unguarded pattern (5-tap stencil, fx0/fx1/fx3/fx4/(fy...)/(fz...) sign flags computed but never passed to wget) and were missed by that fix.

Suggested fix

Same pattern as the flow-module fix in #37: make the wget closures take the neighbour's sign flag, and fall back to replicating the centre voxel's own weight when the neighbour doesn't exist (f == 0), instead of dereferencing an out-of-range offset. Needs a decision on the ghost-weight convention (replicate-nearest is the natural choice, matching what #37 did) and then a straightforward mechanical fix across field/{1d,2d,3d}.h's wget sites, plus the two missed flow bending_jrls sites.

Verification plan

Valgrind (or ASan, if available) run of fastfields-cpu-lib's test_reg_field/test_reg_flow binaries under Zero boundary should go from reporting invalid reads to zero errors, and the bending-order symmetry test should then be extended to cover B_ZERO too.

Workstream: claude-jitfields-to-fastfields

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions