Skip to content

Fix uninitialized memory references - #300

Merged
SimonSapin merged 1 commit into
servo:mainfrom
matteoldani:fix-uninit-memory
Aug 19, 2026
Merged

Fix uninitialized memory references#300
SimonSapin merged 1 commit into
servo:mainfrom
matteoldani:fix-uninit-memory

Conversation

@matteoldani

@matteoldani matteoldani commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #279

This PR resolves a critical memory safety issue in Atom::from_mutated_str.

Previously, the code did &mut *buffer.as_mut_ptr() on a MaybeUninit<[u8; 64]>. Creating a mutable reference to uninitialized memory is instant Undefined Behavior in Rust, and it violated Stacked Borrows (caught by Miri).

This is fixed by:
1. Using raw pointers (buffer.as_mut_ptr()) and std::ptr::copy_nonoverlapping to write to the buffer.
2. Slicing only the initialized portion (slice::from_raw_parts_mut) before creating the &mut str reference, ensuring we never create a reference to uninitialized bytes.

@SimonSapin

SimonSapin commented Aug 19, 2026

Copy link
Copy Markdown
Member

Fixes #260

Does it really? That looks unrelated
Edit: removing so it doesn’t close

@SimonSapin SimonSapin 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.

Unfortunately miri doesn’t seem to find this problem on my machine, but uninitialized &mut [u8; 64] does look wrong so let’s fix it

❯ cargo +nightly miri -V
miri 0.1.0 (e71c0f1e33 2026-08-18)

Comment thread tests/ub_test.rs Outdated
Comment on lines +3 to +8
#[test]
fn test_to_ascii_uppercase() {
let s = DefaultAtom::from("hello");
let upper = s.to_ascii_uppercase();
assert_eq!(&*upper, "HELLO");
}

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.

This file is redundant with test_ascii_lowercase in integration-tests/src/lib.rs, isn’t it?

- Fix uninitialized memory reference UB in from_mutated_str. Creating a
  &mut [u8; 64] reference pointing to uninitialized bytes from MaybeUninit
  violates Rust's validity invariants. Replaced the slice cast with safe
  raw pointer arithmetic (ptr::copy_nonoverlapping) and slice creation
  over only the initialized portion of memory.
- Refactor magic number 64 into a named const MAX_STACK_SIZE.
- Add UB test case.
@SimonSapin
SimonSapin enabled auto-merge August 19, 2026 21:10
@SimonSapin
SimonSapin added this pull request to the merge queue Aug 19, 2026
Merged via the queue into servo:main with commit e67e122 Aug 19, 2026
5 checks passed
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.

Unsoundness in dynamic_set.rs and from_mutated_str

2 participants