fix: v0.4.2 — RELAY ecosystem conformance fixes, pass 3 - #52
Merged
Conversation
- virtual::Bus::publish() (and the default IBus::publish() path) now forces
the classic checksum for diagnostic frame IDs 0x3C/0x3D regardless of the
caller's requested checksum type (LIN 2.2A §2.3.1.5, RELAY §15.3),
matching validate_frame()'s existing rejection of enhanced-checksum
diagnostic frames.
- RELAY adapter LinAdapter::send() now honours the bridged message's
lin.checksum_type (and forces classic for 0x3C/0x3D), routing to
publish_classic() instead of unconditionally upgrading every bridged
frame to enhanced.
- master::Node::run() now returns success (no-op) for an empty schedule
table instead of lin::Errc::invalid_frame, matching RELAY §8.3 ("an
empty table is valid and disables scheduled transmission"). Updated
REQ-MASTER-009/010 requirement text (both req JSON files) to match.
- LDF parser clamps Signal::bit_width to [0, 64] at parse time so
DB::decode()'s bit-extraction loop can't be driven into a shift-by->=64
(UB) by a malformed/adversarial LDF file with an oversized payload.
- CI: SHA-pin ilammy/msvc-dev-cmd and softprops/action-gh-release instead
of floating major-version tags; stop masking cpfusa init / hara init
failures with `|| true` so a broken init hard-fails instead of letting
downstream FuSa/HARA steps run against a stale config.
Regression tests added for all four code fixes, including a malformed-LDF
test that reproduces the bit-width UB under UBSan pre-fix.
Signed-off-by: Matt Jones <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
Gap-audit fix pass against LIN 2.x (LIN Consortium 2.1/2.2A) and RELAY spec, closing four confirmed findings:
cpp-LIN-01 (medium) —
virtual::Bus::publish()emits enhanced checksum for diagnostic framesSpec: LIN 2.2A §2.3.1.5 — "Frame identifiers 60 (0x3C) to 61 (0x3D) shall always use classic checksum" (verified verbatim against the official LIN Consortium 2.2A PDF). RELAY §15.3: diagnostic frames MUST use ClassicChecksum.
do_publish()now forcesChecksumType::Classicwheneverid == kLINDiagRequestID || id == kLINDiagResponseID, regardless of what the caller requested. This brings the production publish path in line withvalidate_frame(), which already rejected an enhanced-checksum diagnosticFrame— the two were previously disagreeing.Applied from the pre-made audit diff (
diffs/cpp-LIN/cpp-LIN-01.diff), which applied cleanly.cpp-LIN-02 (medium) — RELAY adapter discards checksum_type, upgrading bridged frames to enhanced
Spec: LIN 2.2A §2.3.1.5 (as above); RELAY §8.3 (
Bus.PublishvsBus.PublishClassic) and §15.3 (lin.checksum_typemeta must round-trip faithfully).LinAdapter::send()now routes tobus_->publish_classic()whenf.checksum_type == Classicorf.idis a diagnostic ID, and tobus_->publish()otherwise — preserving on-wire checksum fidelity across the RELAY bridge instead of silently re-emitting every bridged frame (including diagnostics) as enhanced.Applied from the pre-made audit diff (
diffs/cpp-LIN/cpp-LIN-02.diff), which applied cleanly.cpp-LIN-A1 (low) —
master::Node::run()returnsinvalid_framefor a spec-valid empty scheduleSpec: RELAY §8.3 (
relay-spec.md:509-511): "SetSchedule installs a new LIN schedule table. An empty table is valid and disables scheduled transmission." (verified directly againstRELAY/spec/relay-spec.md.)set_schedule({})already succeeded (fixed in v0.4.1), butrun()still early-returnedlin::Errc::invalid_frameon an empty schedule — an ERROR, not the no-op the spec requires.run()now returns success ({}) immediately for an empty schedule. Updated the pinnedtests/test_master.cpp:72test (previously asserting the wrong behaviour) and both.fusa-reqs.json/requirements/requirements.jsonentries for REQ-MASTER-009/010, which described the old (incorrect) contract.cpp-LIN-A2 (low, hardening) — LDF
DB::decode()bit-shift width not bounded to [0,64) at parse timeNot a wire-format conformance defect but a defence-in-depth fix:
val |= 1ULL << iin the bit-extraction loop is UB ifi >= 64.bit_widthis parsed directly from untrusted LDF text with no clamp; it was only incidentally bounded by the loop's ownbyte_idx >= data.size()break for realistic (<=8-byte) frames.bit_widthis now clamped to[0, 64]at parse time, independent of payload size.Regression-test proof: added a malformed-LDF test (
Signal bit_width = 999999999) decoded against an oversized (64-byte) payload. Built and ran this test under UBSan (-fsanitize=undefined -fno-sanitize-recover=undefined):runtime error: shift exponent 64 is too large for 64-bit type 'long long unsigned int', SIGABRT.cpp-LIN-A3 (low, CI hygiene) — unpinned third-party actions +
|| true-masked FuSa/HARA stepsactions/*, non-github/*) actions:ilammy/msvc-dev-cmd@v1→@0b201ec7...(v1.13.0),softprops/action-gh-release@v2→@3bb12739...(v2.6.2), resolved viagit ls-remote.|| truefrom thecpfusa init(x2) andcpfusa hara initsteps — these are pure setup steps immediately followed by steps that already gate (cpfusa check), so a silently-failed init should hard-fail rather than let downstream FuSa/HARA reporting run against a missing/stale config.iso26262/iec61508gap-report|| trueand the clang-tidy warning-masking alone: those tools' non-zero exit plausibly signals "gap/warning found" rather than "tool crashed" (gap reports are uploaded as informational artifacts), and tightening warning-gating would require auditing/fixing the existing warning baseline — out of scope for this low-severity, no-diff CI-hygiene item. Noted as a follow-up.Verification
ctest --test-dir build --output-on-failure -j1), matching CI's exact command.publish() on diagnostic IDs forces classic checksumadapt: send preserves classic checksum type across the bridgeadapt: send forces classic checksum for diagnostic frame IDs even when tagged enhancedrun is a no-op success for empty scheduleparse clamps malformed/adversarial signal bit_width to [0,64](UBSan-verified, see above)CHANGELOG.md/ROADMAP.mdupdated per existing convention;CMakeLists.txtversion bumped 0.4.1 → 0.4.2 (self-reported by the CLI viaPROJECT_VERSION, exercised by existingversion_json/version_texttests).cpfusabuild available in this environment (its source repocpp-FuSaisn't checked out locally) — the.fusa-reqs.json/requirements/requirements.jsonREQ-MASTER-009/010 text was updated by hand to match the correctedrun()/set_schedule()contract; CI'scpfusa check/trace/qualifygates will be the first automated verification of the updated safety-requirements artifacts.Not merging — leaving for review per repo policy.