Skip to content

Conversation

@bennetthardwick
Copy link
Contributor

Changelog

  • rust: fix int underflow on chunk with invalid size

Docs

None

Description

When a chunk doesn't close correctly (loss of power, crash, etc.) the size that was written will be u64::MAX. If you try and read a file like this with the Rust reader it breaks because the size of the chunk is expected to be valid. In debug mode this will panic due to underflow, in release mode this value will wrap and break something else down the track.

This change explicitly uses checked calculations for the chunk size and returns an UnexpectedEoc error if any of the calculations overflow.

Fixes #1479.
Fixes FIRE-202.

@linear
Copy link

linear bot commented Nov 4, 2025

let next = reader.next_event().expect("there should be one event");

// test fails with unexpected EOC because sizes are u64::max
assert_matches!(next, Err(McapError::UnexpectedEoc));
Copy link

Choose a reason for hiding this comment

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

Say the file writer flushed the chunk after this call. Would reader.next_event yield data? Or is the reader borked after an error like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general once the reader hits an error it is considered to be in an invalid state. So it will be borked after this.

@nionata
Copy link

nionata commented Nov 4, 2025

Thanks for the quick fix!

@bennetthardwick bennetthardwick merged commit 23b0e1f into main Nov 4, 2025
29 checks passed
@bennetthardwick bennetthardwick deleted the bennett/fix-int-underflow branch November 4, 2025 22:01
@nionata
Copy link

nionata commented Nov 4, 2025

@bennetthardwick when will this be released to crates.io?

@bennetthardwick
Copy link
Contributor Author

@nionata I'll cut a release now 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

rust: Integer overflow with the linear reader

4 participants