Fix RX behavioral defects: reassembly asserts, filter config, callback re-entrancy#260
Merged
Merged
Conversation
…k re-entrancy Adversarial review of the v5 RX pipeline surfaced several behavioral defects, each fixed here with a regression test that fails on the pre-fix code: - rx_subscribe: accumulate filters_dirty instead of overwriting it, so a duplicate subscribe (returning the incumbent) can no longer cancel a hardware acceptance filter reconfiguration requested by a prior subscribe/unsubscribe/set_node_id. - rx_session_accept / rx_session_complete_slot: drop the false (slot->iface_index == ses->iface_index) assertion. Session interface affinity legitimately migrates on a stale&&fresh admission while an in-flight slot at a different priority keeps its own affinity, so the assertion aborted on valid redundant-interface traffic. Per-slot affinity is still enforced at admission. - rx_parse: mask the anonymous UAVCAN v0 message data-type-ID to its 2 low bits ((can_id>>8)&0x3) per the DroneCAN spec; the upper CAN-ID bits carry a random discriminator, not the data-type-ID. rx_filter_for_subscription emits a weak filter for v0 messages with data-type-ID <= 3 so anonymous frames are admitted by hardware, keeping one filter entry per subscription. - rx_session_update: do not dereference the session after dispatching on_message (it may unsubscribe and free the session -> use-after-free). Documented that a callback may unsubscribe itself but must not otherwise reenter the library. - canard_new: mark filters dirty when filter_count > 0 so the forced occupancy filters are programmed on the first poll even before any subscription. - Forced Heartbeat/NodeStatus occupancy dedup now uses a filter-coverage (subset) test (rx_filter_covered) instead of a single representative-frame probe, which could be aliased by a subscription filter and silently suppress the occupancy filter for some priorities/sources. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
canard_new() now takes an iface_bitmap describing the interfaces available on the node. It is ANDed into the requested interface bitmap at the single TX chokepoint (tx_push), so transfers are never queued for interfaces that cannot be drained -- previously requests/responses (which always target all interfaces) and all-publishes wasted heap on undrainable queues. A zero bitmap declares a listen-only node: every enqueue becomes a clean no-op (no spool, no error counter). canard_new rejects a bitmap with bits beyond CANARD_IFACE_COUNT. All call sites, the demo, and the README example are updated. Regression tests cover masking (request/publish/v0), the listen-only no-op, and -- under the leak-detecting intrusive allocator -- refcount/cursor consistency and correct transfer-freeing on the empty-intersection path. Reviewed by Codex (gpt-5.5 xhigh) and Claude Fable 5 (xhigh): no major findings. Also includes minor documentation tweaks (AGENTS.md, README). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
v5 TX/RX hardening in two parts. Every fix ships with a regression test that fails on the pre-fix code and passes after.
RX behavioral fixes (
canard.c)rx_subscribe: accumulatefilters_dirty(||) — a duplicate subscribe no longer cancels a pending acceptance-filter reconfiguration (was silent RX loss on filter-capable nodes).rx_session_accept/rx_session_complete_slot: drop the falseslot->iface_index == ses->iface_indexassert that aborted on valid redundant-interface traffic (per-slot affinity is still enforced at admission).rx_parse(v0 anonymous): route on the 2 low data-type-ID bits per the DroneCAN spec (the upper bits are a random discriminator), with a weak HW acceptance filter for v0 DTID ≤ 3 — still one filter entry per subscription.rx_session_update: dispatchon_messagelast, so a callback that unsubscribes itself can't cause a use-after-free.canard_new: setfilters_dirty = filter_count > 0so forced Heartbeat/NodeStatus occupancy filters program even before the first subscription.rx_filter_covered.Available-interfaces bitmap (intentional
canard_newAPI break)canard_new()gains aniface_bitmapof the interfaces available on the node, ANDed into every enqueue at the single TX chokepoint (tx_push). Requests/responses and all-publishes no longer queue frames for interfaces that can't be drained. A zero bitmap declares a listen-only node (TX is a clean no-op). Bits beyondCANARD_IFACE_COUNTare rejected. Call sites, the demo, and the README example are updated.Verification
Full suite green in the Debug build (assertions on);
clang-tidy+cppcheck+clang-formatclean. Each fix was confirmed to be a real guard by reverting its hunk and watching the test fail — including a refcount/cursor divergence caught under the leak-detecting allocator. The RX and interface changes were each independently reviewed by Codex (gpt-5.5 xhigh) and Claude Fable 5 (xhigh) with no major findings.🤖 Generated with Claude Code