Skip to content

Fix KSChan point-process OOB write of dparam[2] - #3844

Merged
nrnhines merged 2 commits into
masterfrom
fix/kschan-dparam-oob-3843
Aug 12, 2026
Merged

Fix KSChan point-process OOB write of dparam[2]#3844
nrnhines merged 2 commits into
masterfrom
fix/kschan-dparam-oob-3843

Conversation

@nrnhines

Copy link
Copy Markdown
Member

Summary

  • Removes a latent out-of-bounds write in KSChan::alloc that always did prop->dparam[2] = nullptr for point processes.
  • That slot only exists when single-channel mode (or later ion/ligand fields) makes ppsize > 2. Plain point KSChan instances allocate only area and pnt, so index 2 was past the allocated Datum vector.
  • Default ArrayPool freelist order hid the overflow inside the next free slab row; a reversed freelist exposes it under ASAN (see src/nrniv/kschan.cpp is allocator sensitive #3843).
  • nrn_prop_datum_alloc already default-constructs every Datum to the null handle state, and dparam[0]/[1] are filled by point.cpp after nrn_alloc. The redundant nulling is removed; single_->alloc still runs when single_ is set and the singleptr slot is null.

Test plan

  • Rebuild and run hoctests::test_kschan_py (with and without ASAN).
  • Optionally re-apply the reverse freelist seed in arraypool.h from src/nrniv/kschan.cpp is allocator sensitive #3843 and confirm ASAN no longer fails on that test.
  • Exercise single-channel paths in the same test (ks.single(1), etc.) to confirm singleptr allocation still works.

Fixes #3843

KSChan::alloc always nulled prop->dparam[2] for point processes, but
that slot exists only when single-channel mode (or later fields) makes
ppsize > 2. Plain point KSChan instances allocate only area and pnt, so
the write was past the allocated Datum vector. The default ArrayPool
freelist order hid the overflow inside the next free slab row; a
reversed freelist exposes it under ASAN.

nrn_prop_datum_alloc already default-constructs every Datum to null, and
dparam[0]/[1] are filled by point.cpp after nrn_alloc. The redundant
nulling is removed; single_->alloc still runs when single_ is set and
the singleptr slot is null.
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

✔️ aa65940 -> artifacts URL

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.35%. Comparing base (2a21b48) to head (aa65940).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3844      +/-   ##
==========================================
+ Coverage   68.33%   68.35%   +0.02%     
==========================================
  Files         700      700              
  Lines      112198   112196       -2     
==========================================
+ Hits        76668    76692      +24     
+ Misses      35530    35504      -26     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@azure-pipelines

Copy link
Copy Markdown

✔️ aa65940 -> Azure artifacts URL

@nrnhines
nrnhines requested a review from mgeplf August 11, 2026 20:16

@mgeplf mgeplf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@nrnhines
nrnhines merged commit d71477f into master Aug 12, 2026
44 checks passed
@nrnhines
nrnhines deleted the fix/kschan-dparam-oob-3843 branch August 12, 2026 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

src/nrniv/kschan.cpp is allocator sensitive

2 participants