Skip to content

Arc/Rc is creating references to uninitialized values #119241

Closed
@taiki-e

Description

@taiki-e

addr_of_mut doc says:

Creating a reference with &/&mut is only allowed if the pointer is properly aligned and points to initialized data. For cases where those requirements do not hold, raw pointers should be used instead. However, &mut expr as *mut _ creates a reference before casting it to a raw pointer, and that reference is subject to the same rules as all other references. This macro can create a raw pointer without creating a reference first.

let mut uninit = MaybeUninit::<Demo>::uninit();
// `&uninit.as_mut().field` would create a reference to an uninitialized `bool`,
// and thus be Undefined Behavior!
let f1_ptr = unsafe { ptr::addr_of_mut!((*uninit.as_mut_ptr()).field) };

However, some code in Arc/Rc seems to create references to uninitialized values.

For example, in the following code, inner is a pointer to an uninitialized ArcInner, but Layout::for_value(&*inner) creates a reference to it, and &mut (*inner).strong and &mut (*inner).weak creates references to (uninitialized) counters.

rust/library/alloc/src/sync.rs

Lines 1825 to 1836 in 495203b

unsafe fn initialize_arcinner(
ptr: NonNull<[u8]>,
layout: Layout,
mem_to_arcinner: impl FnOnce(*mut u8) -> *mut ArcInner<T>,
) -> *mut ArcInner<T> {
let inner = mem_to_arcinner(ptr.as_non_null_ptr().as_ptr());
debug_assert_eq!(unsafe { Layout::for_value(&*inner) }, layout);
unsafe {
ptr::write(&mut (*inner).strong, atomic::AtomicUsize::new(1));
ptr::write(&mut (*inner).weak, atomic::AtomicUsize::new(1));
}

Such codes can also be found in some other places in Arc/Rc.

ptr::copy_nonoverlapping(vec_ptr, &mut (*rc_ptr).data as *mut [T] as *mut T, len);

&mut (*ptr).data as *mut _ as *mut u8,

ptr::copy_nonoverlapping(v.as_ptr(), &mut (*ptr).data as *mut [T] as *mut T, v.len());

let elems = &mut (*ptr).data as *mut [T] as *mut T;

rust/library/alloc/src/rc.rs

Lines 1888 to 1890 in 495203b

debug_assert_eq!(Layout::for_value(&*inner), layout);
ptr::write(&mut (*inner).strong, Cell::new(1));

&mut (*ptr).value as *mut _ as *mut u8,

ptr::copy_nonoverlapping(v.as_ptr(), &mut (*ptr).value as *mut [T] as *mut T, v.len());

let elems = &mut (*ptr).value as *mut [T] as *mut T;

ptr::copy_nonoverlapping(vec_ptr, &mut (*rc_ptr).value as *mut [T] as *mut T, len);

At least the following code should run into this issue because it calls Arc::initialize_arcinner via <Arc<_> as From<Box<_>>::from -> Arc::from_box_in -> Arc::allocate_for_ptr_in -> Arc::allocate_for_layout.

use std::{boxed::Box, sync::Arc};
let boxed: Box<str> = Box::from("abc");
let arc: Arc<str> = Arc::from(boxed);
assert_eq!("arc", &arc[..]);

Solution

This can be fixed in the following ways:

  • Use Layout::for_value_raw (unstable) instead of Layout::for_value.
  • Use addr_of_mut for code creating reference to uninitialized conters (.strong/.weak).
  • Use addr_of_mut or new_uninit/new_unint_slice for code creating reference to uninitialized data (.data/.value). (I think the latter would be clearer.)

cc @RalfJung: Miri doesn't seem to report this as UB, but is my understanding that this is the kind of UB that Miri does not currently support detection correct? Or is it missed or allowed for some reason?

Metadata

Metadata

Assignees

Labels

A-atomicArea: Atomics, barriers, and sync primitivesC-cleanupCategory: PRs that clean code up or issues documenting cleanup.C-discussionCategory: Discussion or questions that doesn't represent real issues.T-opsemRelevant to the opsem team

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions