dangernoodle-io / breadboard / 27525665174

100%

push

github

fix(bb_event): hold subscriber walk safe against concurrent unsubscribe (SMP UAF, telemetry review P0) (#444)

the race: bb_event_common_dispatch snapshotted only sub_head under the port
lock, then released the lock before walking sub->next while calling each
subscriber callback. on dual-core esp32-s3 (tdongle-s3, r4_wifis3) a
concurrent bb_event_unsubscribe — e.g. bb_event_ring_detach on SSE-client
disconnect from the httpd task — could free a subscriber node mid-walk,
causing the walker to read sub->next from freed/recycled pool memory
(UAF / corrupted traversal). benign on single-core c3/s2 but a real data
race on SMP.

chosen fix: hold the recursive port lock across the entire subscriber walk.
both the host pthread backend (PTHREAD_MUTEX_RECURSIVE) and the ESP-IDF
FreeRTOS backend (xSemaphoreCreateRecursiveMutex) use recursive mutexes,
so callbacks that re-acquire the lock (ring_capture calls bb_event_lock())
are safe.

callback analysis:
- ring_capture: calls bb_event_lock() recursively (safe); no I/O; does NOT
  call bb_event_unsubscribe during dispatch.
- capture_cb (bb_event_routes): acquires s_topics_lock and c->port_lock —
  both separate from the port mutex, no deadlock; no I/O; does NOT
  call bb_event_unsubscribe during dispatch.

no callback unsubscribes-self or another subscriber during normal dispatch,
and none blocks on I/O, so holding the lock across the walk is safe.
the full-chain-snapshot alternative was considered but is unnecessary here
because the recursive-mutex property already makes the simpler approach safe.

regression test: test_bb_event_dispatch_unsubscribe_during_walk_is_safe —
exercises unsubscribe-during-dispatch by having sub_a's callback call
bb_event_unsubscribe on sub_b (a different subscriber on the same topic)
while the dispatcher holds the recursive lock. asserts: walk completes
without crash (ASan/valgrind catch UAF on CI), sub_a ran exactly once,
sub_b receives no further events after unsubscribe takes effect... (continued)

1615 of 1615 branches covered (100.0%)

Branch coverage included in aggregate %.

1 of 1 new or added line in 1 file covered. (100.0%)

2702 of 2702 relevant lines covered (100.0%)

1175.52 hits per line