Skip to content

Commit 492ef76

Browse files
committed
patina_internal_collections: Fix UB memory read in Node fields
Fixes an undefined behavior issue where Cell::set() reads uninitialized memory during linked list creation in Storage::resize(). Root Cause: - Cell::set() internally uses mem::replace(), which reads the old value before writing the new one. - When Storage::resize() allocates new nodes and calls build_linked_list(), the Cell fields contain uninitialized memory. - Reading uninitialized memory is undefined behavior, even if immediately overwritten. Unwanted compiler "optimizations" could follow. Impact: - Any package using patina_internal_collections. - Potential memory corruption and non-deterministic errors - Detected by Miri testing (issue #560) Fix: - Initialize all Cell fields using ptr::write() before build_linked_list() - Use addr_of_mut!() to get field pointers without creating references to uninitialized data - Verified with Miri Introduced by: - PR #1050 (Nov 13, 2025) Replace AtomicPtr with Cell in patina_internal_collections - AtomicPtr::store() writes without reading the old value, but Cell::set() uses mem::replace() which reads before writing Testing: - Tested with: cargo +nightly-2025-09-19 miri test -p patina_dxe_core. - 7 tests now pass (previously 0/469 due to this UB issue). Related to #560
1 parent 939804a commit 492ef76

File tree

1 file changed

+25
-0
lines changed
  • core/patina_internal_collections/src

1 file changed

+25
-0
lines changed

core/patina_internal_collections/src/node.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,31 @@ where
204204
// When current capacity is 0, we just need to copy the data and build the available list
205205
if self.capacity() == 0 {
206206
self.data = buffer;
207+
208+
// Initialize all Cell fields to prevent reading uninitialized memory.
209+
// Cell::set() internally uses mem::replace() which reads the old value before writing.
210+
// We use ptr::write to initialize the fields without creating references to uninitialized data.
211+
for i in 0..self.data.len() {
212+
// SAFETY: node_ptr is derived from self.data which is a valid slice with length i+1.
213+
// We use addr_of_mut! to get field pointers without creating intermediate references
214+
// to uninitialized data, then ptr::write to initialize the fields.
215+
unsafe {
216+
let node_ptr = self.data.as_mut_ptr().add(i);
217+
218+
let color_ptr = core::ptr::addr_of_mut!((*node_ptr).color);
219+
core::ptr::write(color_ptr, Cell::new(BLACK));
220+
221+
let parent_ptr = core::ptr::addr_of_mut!((*node_ptr).parent);
222+
core::ptr::write(parent_ptr, Cell::new(core::ptr::null_mut::<Node<D>>()));
223+
224+
let left_ptr = core::ptr::addr_of_mut!((*node_ptr).left);
225+
core::ptr::write(left_ptr, Cell::new(core::ptr::null_mut::<Node<D>>()));
226+
227+
let right_ptr = core::ptr::addr_of_mut!((*node_ptr).right);
228+
core::ptr::write(right_ptr, Cell::new(core::ptr::null_mut::<Node<D>>()));
229+
}
230+
}
231+
207232
Self::build_linked_list(self.data);
208233
self.available.set(self.data[0].as_mut_ptr());
209234
return;

0 commit comments

Comments
 (0)