From PR #844 review (Copilot):
Serial serialize writes byte_count into the header without applying SerialEncoding stuffing. Since byte_count can legitimately be 0x7D or 0x7E (125/126 decoded bytes), this can inject a raw escape/flag byte into the header on the wire, which is likely to break framing for receivers that use 0x7E delimiters. Consider stuffing the header bytes too (and decoding them correspondingly) or otherwise guaranteeing these values cannot occur on the wire.
Location: mctp-rs/src/medium/serial.rs:278
Deliberate design (today): the in-tree code carries the upstream author's decision to mirror SmbusEspiMedium's pattern of emitting the 2-byte header (SERIAL_REVISION, byte_count) raw. This is captured in an inline source comment:
Header: revision + byte_count emitted directly (NOT stuffed), matching SmbusEspiMedium's header pattern. ... when byte_count happens to equal 0x7E or 0x7D — round-trips cleanly through this implementation's deserialize.
Strict-conformance gap: DSP0253 §7.1 specifies that ALL bytes between flag delimiters (0x7E) must be byte-stuffed. The current implementation:
- Self-roundtrips correctly (the in-tree
MctpSerialMedium::deserialize reads the 2 header bytes raw, matching serialize).
- Mis-frames against strict-DSP0253 receivers when
body_wire_len ∈ {125, 126} — a raw 0x7D or 0x7E would be interpreted as escape or end-flag respectively.
Practical exposure: the serial feature is opt-in and no embedded-services consumer enables it today. The exposure surfaces only when (a) serial is enabled AND (b) the peer is a strict-DSP0253 implementation other than this one.
Fix sketch: byte-stuff the 2-byte header via SerialEncoding::write_byte (matching the FCS pattern at the bottom of serialize) and un-stuff in deserialize. Update the inline source comment + golden fixtures.
Scope: out of scope for the source-bump PR #844 (verbatim port of upstream design); follow-up against the in-tree source.
From PR #844 review (Copilot):
Location:
mctp-rs/src/medium/serial.rs:278Deliberate design (today): the in-tree code carries the upstream author's decision to mirror
SmbusEspiMedium's pattern of emitting the 2-byte header (SERIAL_REVISION,byte_count) raw. This is captured in an inline source comment:Strict-conformance gap: DSP0253 §7.1 specifies that ALL bytes between flag delimiters (
0x7E) must be byte-stuffed. The current implementation:MctpSerialMedium::deserializereads the 2 header bytes raw, matching serialize).body_wire_len ∈ {125, 126}— a raw0x7Dor0x7Ewould be interpreted as escape or end-flag respectively.Practical exposure: the
serialfeature is opt-in and noembedded-servicesconsumer enables it today. The exposure surfaces only when (a)serialis enabled AND (b) the peer is a strict-DSP0253 implementation other than this one.Fix sketch: byte-stuff the 2-byte header via
SerialEncoding::write_byte(matching the FCS pattern at the bottom ofserialize) and un-stuff indeserialize. Update the inline source comment + golden fixtures.Scope: out of scope for the source-bump PR #844 (verbatim port of upstream design); follow-up against the in-tree source.