Skip to content

net_mana: Added validation for ContiguousBufferMemory construction#2944

Merged
ben-zen merged 6 commits intomicrosoft:mainfrom
ben-zen:validate_ring_buffer
Mar 20, 2026
Merged

net_mana: Added validation for ContiguousBufferMemory construction#2944
ben-zen merged 6 commits intomicrosoft:mainfrom
ben-zen:validate_ring_buffer

Conversation

@ben-zen
Copy link
Copy Markdown
Contributor

@ben-zen ben-zen commented Mar 11, 2026

ContiguousBufferMemory works by having a rolling len-sized window of valid offsets across all u32 values. The only safe window sizes are therefore powers of 2, otherwise you can potentially have multiple buffers assigned the same offset near the end of the u32 space, due to how modular arithmetic rolls over values.

@ben-zen ben-zen requested a review from a team as a code owner March 11, 2026 17:29
Copilot AI review requested due to automatic review settings March 11, 2026 17:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds input validation to ContiguousBufferManager construction to ensure ring-buffer indexing remains correct across u32 wrap-around, and introduces a unit test to enforce the allowed page_limit values.

Changes:

  • Add a constructor-time validation for page_limit to prevent invalid ring sizing.
  • Add a unit test that checks only power-of-two page counts are accepted.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread vm/devices/net/net_mana/src/lib.rs Outdated
Comment thread vm/devices/net/net_mana/src/lib.rs Outdated
Comment thread vm/devices/net/net_mana/src/lib.rs
@ben-zen ben-zen force-pushed the validate_ring_buffer branch from a5a0065 to 11ca577 Compare March 11, 2026 18:38
Copilot AI review requested due to automatic review settings March 11, 2026 20:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.


You can also share your feedback on Copilot code review. Take the survey.

Comment thread vm/devices/net/net_mana/src/lib.rs
Comment thread vm/devices/net/net_mana/src/lib.rs
Comment thread vm/devices/net/net_mana/src/lib.rs
Copy link
Copy Markdown
Contributor

@erfrimod erfrimod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ben-zen ben-zen changed the title Added validation for ContiguousBufferMemory construction net_mana: Added validation for ContiguousBufferMemory construction Mar 12, 2026
Copilot AI review requested due to automatic review settings March 17, 2026 17:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@ben-zen ben-zen merged commit dcb7c19 into microsoft:main Mar 20, 2026
59 of 60 checks passed
moor-coding pushed a commit to moor-coding/openvmm that referenced this pull request Mar 24, 2026
…icrosoft#2944)

ContiguousBufferMemory works by having a rolling `len`-sized window of
valid offsets across all `u32` values. The only safe window sizes are
therefore powers of 2, otherwise you can potentially have multiple
buffers assigned the same offset near the end of the `u32` space, due to
how modular arithmetic rolls over values.

---------

Co-authored-by: Ben Lewis <Ben.Lewis@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants