OpenDevicePartnership / OpenDevicePartnership/embedded-services

[mctp-rs] SmbusEspiMedium::serialize: bounds-check body_wire_len before truncating to byte_count: u8

Open Beginner friendly
#846 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
23
Forks
53
Avg merge
2d 20h
Merged PRs (30d)
13

Description

From PR #844 review (Copilot):

byte_count is set via body_wire_len as u8 with no bounds check. If a caller writes more than 255 bytes into the body region, this will truncate byte_count and produce an invalid SMBus frame / mismatched PEC. Even if higher layers usually cap this via max_message_body_size(), it's safer for the medium to validate body_wire_len <= u8::MAX and return an error otherwise.

Location: mctp-rs/src/medium/smbus_espi.rs:91

Pre-existing: verified — same as u8 truncation exists at the v0.2.0 baseline (dymk/mctp-rs @ 3d941ba, line is byte_count: body_len as u8). PR #844 only renames body_lenbody_wire_len; behavior unchanged.

SMBus context: the SMBus spec hard-caps a frame's byte_count field to 1 byte (255). The MCTP-over-SMBus layer above should fragment messages so individual packet bodies stay ≤ 255 bytes. Today, the cap is implicit (via max_message_body_size() returning a workspace-wide MTU); a defensive check at the medium boundary would convert silent corruption into an explicit MctpPacketError::MediumError.

Fix sketch:

let byte_count_u8 = u8::try_from(body_wire_len)
    .map_err(|_| MctpPacketError::MediumError("body_wire_len exceeds SMBus byte_count u8 max"))?;

Scope: out of scope for the source-bump PR #844 (pre-existing behavior); follow-up against the in-tree source.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at mctp-rs/src/medium/smbus_espi.rs:91 and inspect SmbusEspiMedium::serialize, especially how body_wire_len becomes byte_count. Add the bounds validation described in the issue so oversized bodies return MctpPacketError::MediumError instead of truncating, then verify that normal frames still serialize and oversized bodies are rejected.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
embedded-iot
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.