fix: reject oversized payloads at LIN publish paths; correct E2E manual - #26
Open
SoundMatt wants to merge 1 commit into
Open
fix: reject oversized payloads at LIN publish paths; correct E2E manual#26SoundMatt wants to merge 1 commit into
SoundMatt wants to merge 1 commit into
Conversation
SAFETY_MANUAL.md documented the canonical ASIL-B E2E-protected publish pattern as safety::Protector::protect() output fed directly into Bus::do_publish()/slave::Node::set_response(). protect() always prepends a fixed kHeaderSize (10) byte header, so its output is always >= 10 bytes -- but kLINMaxDataLen (8) already caps a LIN frame's data field below that, for any payload including a zero-byte one. None of the actual publish paths validated data.size() at all, so the documented "safe" pattern silently produced out-of-spec frames instead of failing loudly. Fixes: - Bus::do_publish() now rejects any non-empty payload longer than kLINMaxDataLen with ErrInvalidFrame (empty payload keeps its existing "unregister this ID" meaning and is unaffected). slave::Node:: set_response() delegates to do_publish() and is covered transitively. - The RELAY adapter's LinAdapter::send() translates that same condition to ErrPayloadTooLarge before it would otherwise leak lin::Errc::invalid_frame through relay::INode::send(), whose documented contract (spec §10.1) only allows ErrClosed, ErrNotConnected, ErrTimeout, or ErrPayloadTooLarge. - SAFETY_MANUAL.md §4.2 no longer presents the broken single-frame pattern as usable. It now states the real numeric constraint, explains why no payload size makes it fit, and documents that Protector/Receiver are for use over a transport capable of carrying more than 8 bytes per logical message (e.g. a future multi-frame LIN diagnostic transport -- not yet implemented, see ROADMAP.md) rather than direct single-frame publish -- with a corrected example that never calls Bus::publish()/set_response() on protected output. Closes #17 Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.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
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
SAFETY_MANUAL.mddocumented the canonical ASIL-B E2E-protected publishpattern as
Protector::protect()output fed directly intoBus::do_publish()/slave::Node::set_response().protect()alwaysprepends a fixed
kHeaderSize(10) byte header, which already exceedskLINMaxDataLen(8) for any payload, including a zero-byte one — so thedocumented pattern can never actually fit in a single LIN frame. None of
the real publish paths validated payload length at all, so this silently
produced out-of-spec frames instead of failing loudly.
Bus::do_publish()now rejects any non-empty payload longer thankLINMaxDataLenwithErrInvalidFrame(an empty payload keeps itsexisting "unregister this ID" meaning).
slave::Node::set_response()delegates to
do_publish()and is covered transitively.LinAdapter::send()translates that same condition toErrPayloadTooLarge—relay::INode::send()'s documented contract(spec §10.1) only allows
ErrClosed/ErrNotConnected/ErrTimeout/ErrPayloadTooLarge, so it can't just forwardIBus'slin::Errc::invalid_frameraw (mirrors the existing precedent forinvalid-ID handling right above it).
SAFETY_MANUAL.md§4.2 no longer presents the broken pattern as usable.It states the real numeric constraint, explains why no payload size makes
it fit, and documents that
Protector/Receiverare for use over atransport that can carry more than 8 bytes per logical message (e.g. a
future multi-frame LIN diagnostic transport — not yet implemented, see
ROADMAP.md) rather than direct single-frame publish, with a correctedexample that never calls
Bus::publish()/set_response()on protectedoutput.
Closes #17
Test plan
ctest— 175/175 pass (4 new regression tests acrosstest_virtual.cpp,test_slave.cpp,test_relay_adapter.cpp)relay conform --strict/relay interop --protocol LIN— both PASS