Skip to content

Fix allocator capacity bounds checking and tracking in Buf32#754

Merged
simonwuelker merged 1 commit into
servo:mainfrom
matteoldani:fix-buf32
Jul 8, 2026
Merged

Fix allocator capacity bounds checking and tracking in Buf32#754
simonwuelker merged 1 commit into
servo:mainfrom
matteoldani:fix-buf32

Conversation

@matteoldani

Copy link
Copy Markdown
Contributor

This PR fixes a memory safety vulnerability in Buf32 that causes Undefined Behavior when interacting with global allocators that over-provision memory (such as jemalloc ).

When Buf32 initially allocates memory via Vec::with_capacity() , the global allocator often returns a buffer slightly larger than requested to align with memory chunk boundaries. Previously, Buf32 mistakenly stored the requested capacity rather than the actual capacity returned by the allocator.

When the buffer was later deallocated or resized using Vec::from_raw_parts , it passed this smaller, incorrect capacity back to the global allocator. Providing a mismatched layout size back to the allocator violates Vec 's API invariants and is
Undefined Behavior, which can result in immediate segmentation faults or silent heap corruption.

Changes:

Buf32::with_capacity now queries vec.capacity() to determine the true allocated layout size.
• The capacity calculation translates this back into byte capacity by explicitly subtracting the first H header block.
• Adds a bounds check against u32::MAX to explicitly panic and prevent 32-bit truncation if an allocation happens to unexpectedly exceed u32::MAX bytes.

Resolves #751

@matteoldani

Copy link
Copy Markdown
Contributor Author

I've realized that the grow function suffered of the same issue

@github-actions github-actions Bot added V-non-breaking A non-breaking change and removed V-non-breaking A non-breaking change labels Jul 6, 2026
Comment thread tendril/src/buf32.rs Outdated
self.cap = new_cap;

let actual_vec_cap = vec.capacity();
let actual_cap_bytes = (actual_vec_cap - 1) * mem::size_of::<H>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have bytes_to_vec_capacity, can we add a vec_capacity_to_bytes to avoid duplicating this code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks for the suggestions

Comment thread tendril/src/buf32.rs Outdated
let actual_vec_cap = vec.capacity();
let actual_cap_bytes = (actual_vec_cap - 1) * mem::size_of::<H>();
if actual_cap_bytes > std::u32::MAX as usize {
panic!("tendril: capacity overflow");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Use the OFLOW constant.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@github-actions github-actions Bot added V-non-breaking A non-breaking change and removed V-non-breaking A non-breaking change labels Jul 8, 2026

@simonwuelker simonwuelker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks

@simonwuelker simonwuelker added this pull request to the merge queue Jul 8, 2026
Merged via the queue into servo:main with commit 5f3d8a4 Jul 8, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

V-non-breaking A non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buf32 allocator capacity mismatch triggers Undefined Behavior upon reallocation/drop

2 participants