Skip to content

Commit d55851a

Browse files
committed
Fix Point_process SoA UAF on free and NetCon weight ctor dual-write
ASan CI: nrn_point_process_soa_sync during free_one_point/relocate read freed Prop or dangling state. Stop syncing on free (SoA row released in ~Point_process); harden sync to verify prop still owns this pnt and that _vnt is a live NrnThread. NetCon factory wrote magnitude only to weight_ heap; HOC weight[] reads SoA and init_events soa_to_heap wiped the value (0.0 != 0.1). Mirror magnitude into Weight SoA after construction.
1 parent 15b4db3 commit d55851a

2 files changed

Lines changed: 28 additions & 9 deletions

File tree

src/nrncvode/netcvode.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4903,6 +4903,13 @@ NetCon* NetCvode::install_deliver(neuron::container::data_handle<double> dsrc,
49034903
NetCon* d = new NetCon(ps, target);
49044904
d->delay_ = delay;
49054905
d->weight_[0] = magnitude;
4906+
// Dual-write: constructor arg is written to heap; HOC weight[] reads SoA.
4907+
// INITIAL (init_events) does soa_to_heap then heap_to_soa — SoA must already
4908+
// hold magnitude or weight[0] is wiped to 0.
4909+
if (!d->weight_soa_.empty()) {
4910+
d->weight_soa_[0].value() = magnitude;
4911+
}
4912+
d->soa_sync();
49064913
structure_change_cnt_ = 0;
49074914
return d;
49084915
}

src/nrnoc/point.cpp

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,27 @@ void nrn_point_process_soa_sync(Point_process* pnt) {
6060
if (!pnt || !pnt->_soa_id) {
6161
return;
6262
}
63+
// Owner must still be alive (owning_handle in g_point_process_soa_owners).
64+
if (!g_point_process_soa_owners.count(pnt)) {
65+
return;
66+
}
6367
auto h = neuron::container::network::point_process_soa(pnt);
64-
if (pnt->prop) {
65-
h.mech_type() = pnt->prop->_type;
66-
// Prop for a point process always owns a mechanism SoA row.
67-
h.instance() = static_cast<int>(pnt->prop->current_row());
68+
// Only read Prop* when non-null and still linked to this Point_process.
69+
// During free/relocate, prop may already be deleted or half-torn-down.
70+
Prop* p = pnt->prop;
71+
if (p && p->dparam && p->dparam[1].get<Point_process*>() == pnt) {
72+
h.mech_type() = p->_type;
73+
// Prop for a point process owns a mechanism SoA row while live.
74+
h.instance() = static_cast<int>(p->current_row());
6875
} else {
6976
h.mech_type() = -1;
7077
h.instance() = -1;
7178
}
72-
if (pnt->_vnt) {
73-
h.thread_id() = static_cast<NrnThread*>(pnt->_vnt)->id;
79+
// _vnt is NrnThread* when set; only use if it looks like a live thread.
80+
auto* nt = static_cast<NrnThread*>(pnt->_vnt);
81+
if (nt && nrn_threads && nrn_nthread > 0 && nt >= nrn_threads &&
82+
nt < nrn_threads + nrn_nthread) {
83+
h.thread_id() = nt->id;
7484
} else {
7585
h.thread_id() = -1;
7686
}
@@ -359,10 +369,11 @@ void connect_point_process_pointer(void) {
359369
static void free_one_point(Point_process* pnt) {
360370
auto* p = pnt->prop;
361371
if (!p) {
362-
nrn_point_process_soa_sync(pnt);
372+
// SoA row is released in ~Point_process; do not touch Prop/_vnt here.
373+
pnt->_vnt = nullptr;
363374
return;
364375
}
365-
if (!nrn_is_artificial_[p->_type]) {
376+
if (!nrn_is_artificial_[p->_type] && pnt->node) {
366377
auto* p1 = pnt->node->prop;
367378
if (p1 == p) {
368379
pnt->node->prop = p1->next;
@@ -387,11 +398,12 @@ static void free_one_point(Point_process* pnt) {
387398
delete p;
388399
pnt->prop = (Prop*) 0;
389400
pnt->node = (Node*) 0;
401+
pnt->_vnt = nullptr;
390402
if (pnt->sec) {
391403
section_unref(pnt->sec);
392404
}
393405
pnt->sec = (Section*) 0;
394-
nrn_point_process_soa_sync(pnt);
406+
// No soa_sync: reading prop/_vnt after teardown is UAF (ASan). Row freed in ~Point_process.
395407
}
396408

397409
// called from prop_free

0 commit comments

Comments
 (0)