Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Fix - Uses std::mem::transmute and std::ptr::write in unsafe code in append_vec.rs #32711

Merged
merged 1 commit into from
Aug 4, 2023
Merged

Fix - Uses std::mem::transmute and std::ptr::write in unsafe code in append_vec.rs #32711

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions runtime/src/append_vec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,11 +702,13 @@ pub mod tests {
}

impl AppendVecStoredAccountMeta<'_> {
#[allow(clippy::cast_ref_to_mut)]
fn set_data_len_unsafe(&self, new_data_len: u64) {
// UNSAFE: cast away & (= const ref) to &mut to force to mutate append-only (=read-only) AppendVec
unsafe {
*(&self.meta.data_len as *const u64 as *mut u64) = new_data_len;
std::ptr::write(
std::mem::transmute::<*const u64, *mut u64>(&self.meta.data_len),
new_data_len,
);
Comment on lines +708 to +711
Copy link

Choose a reason for hiding this comment

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

This is still UB. Casting/transmuting a immutable reference to a mutable reference is always UB. The only sound way of mutating an immutable reference to a mutable reference is to use a Cell or UnsafeCell.

See this playground example for proof with Miri.

error: Undefined Behavior: attempting a write access using <1679> at alloc852[0x0], but that tag only grants SharedReadOnly permission for this location
  --> src/main.rs:7:9
   |
7  | /         std::ptr::write(
8  | |             std::mem::transmute::<*const u64, *mut u64>(&data_len),
9  | |             new_data_len,
10 | |         );
   | |         ^
   | |         |
   | |_________attempting a write access using <1679> at alloc852[0x0], but that tag only grants SharedReadOnly permission for this location
   |           this error occurs as part of an access at alloc852[0x0..0x8]
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <1679> was created by a SharedReadOnly retag at offsets [0x0..0x8]
  --> src/main.rs:8:57
   |
8  |             std::mem::transmute::<*const u64, *mut u64>(&data_len),
   |                                                         ^^^^^^^^^
   = note: BACKTRACE (of the first span):
   = note: inside `main` at src/main.rs:7:9: 10:10

NOTE: The current cast_ref_to_mut or the now uplifted invalid_reference_casting lint unfortunately do not (yet) lint against, but I'm planning on fixing it.

}
}

Expand All @@ -717,11 +719,13 @@ pub mod tests {
executable_byte
}

#[allow(clippy::cast_ref_to_mut)]
fn set_executable_as_byte(&self, new_executable_byte: u8) {
// UNSAFE: Force to interpret mmap-backed &bool as &u8 to write some crafted value;
unsafe {
*(&self.account_meta.executable as *const bool as *mut u8) = new_executable_byte;
std::ptr::write(
std::mem::transmute::<*const bool, *mut u8>(&self.account_meta.executable),
new_executable_byte,
);
}
}
}
Expand Down