Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audit types used by VM due to rust 1.68 memory layout changes #29852

Closed
brooksprumo opened this issue Jan 24, 2023 · 1 comment · Fixed by #30192
Closed

Audit types used by VM due to rust 1.68 memory layout changes #29852

brooksprumo opened this issue Jan 24, 2023 · 1 comment · Fixed by #30192
Assignees
Labels
runtime Issues related to runtime, BPF, and LLVM

Comments

@brooksprumo
Copy link
Contributor

Problem

Rust 1.68 has changed memory layout of certain types, which impacts Vec and other types within the codebase. This is an issue for any types that are used within the VM where its memory layout is assumed.

This was discovered when upgrading Rust to 1.66.0, which also upgraded nightly to 1.68. The check_type_assumptions test failed due to the layout of Instruction and AccountInfo changing.

pub fn check_type_assumptions() {
extern crate memoffset;
use {
crate::{clock::Epoch, instruction::AccountMeta},
memoffset::offset_of,
std::{
cell::RefCell,
mem::{align_of, size_of},
rc::Rc,
str::FromStr,
},
};
// Code in this file assumes that u64 and usize are the same
assert_eq!(size_of::<u64>(), size_of::<usize>());
// Code in this file assumes that u8 is byte aligned
assert_eq!(1, align_of::<u8>());
// Enforce Instruction layout
{
assert_eq!(size_of::<AccountMeta>(), 32 + 1 + 1);
let pubkey1 = Pubkey::from_str("J9PYCcoKusHyKRMXnBL17VTXC3MVETyqBG2KyLXVv6Ai").unwrap();
let pubkey2 = Pubkey::from_str("Hvy4GHgPToZNoENTKjC4mJqpzWWjgTwXrFufKfxYiKkV").unwrap();
let pubkey3 = Pubkey::from_str("JDMyRL8rCkae7maCSv47upNuBMFd3Mgos1fz2AvYzVzY").unwrap();
let account_meta1 = AccountMeta {
pubkey: pubkey2,
is_signer: true,
is_writable: false,
};
let account_meta2 = AccountMeta {
pubkey: pubkey3,
is_signer: false,
is_writable: true,
};
let data = vec![1, 2, 3, 4, 5];
let instruction = Instruction {
program_id: pubkey1,
accounts: vec![account_meta1.clone(), account_meta2.clone()],
data: data.clone(),
};
let instruction_addr = &instruction as *const _ as u64;
// program id
assert_eq!(offset_of!(Instruction, program_id), 48);
let pubkey_ptr = (instruction_addr + 48) as *const Pubkey;
unsafe {
assert_eq!(*pubkey_ptr, pubkey1);
}
// accounts
assert_eq!(offset_of!(Instruction, accounts), 0);
let accounts_ptr = (instruction_addr) as *const *const AccountMeta;
let accounts_cap = (instruction_addr + 8) as *const usize;
let accounts_len = (instruction_addr + 16) as *const usize;
unsafe {
assert_eq!(*accounts_cap, 2);
assert_eq!(*accounts_len, 2);
let account_meta_ptr = *accounts_ptr;
assert_eq!(*account_meta_ptr, account_meta1);
assert_eq!(*(account_meta_ptr.offset(1)), account_meta2);
}
// data
assert_eq!(offset_of!(Instruction, data), 24);
let data_ptr = (instruction_addr + 24) as *const *const [u8; 5];
let data_cap = (instruction_addr + 24 + 8) as *const usize;
let data_len = (instruction_addr + 24 + 16) as *const usize;
unsafe {
assert_eq!(*data_cap, 5);
assert_eq!(*data_len, 5);
let u8_ptr = *data_ptr;
assert_eq!(*u8_ptr, data[..]);
}
}
// Enforce AccountInfo layout
{
let key = Pubkey::from_str("6o8R9NsUxNskF1MfWM1f265y4w86JYbEwqCmTacdLkHp").unwrap();
let mut lamports = 31;
let mut data = vec![1, 2, 3, 4, 5];
let owner = Pubkey::from_str("2tjK4XyNU54XdN9jokx46QzLybbLVGwQQvTfhcuBXAjR").unwrap();
let account_info = AccountInfo {
key: &key,
is_signer: true,
is_writable: false,
lamports: Rc::new(RefCell::new(&mut lamports)),
data: Rc::new(RefCell::new(&mut data)),
owner: &owner,
executable: true,
rent_epoch: 42,
};
let account_info_addr = &account_info as *const _ as u64;
// key
assert_eq!(offset_of!(AccountInfo, key), 0);
let key_ptr = (account_info_addr) as *const &Pubkey;
unsafe {
assert_eq!(**key_ptr, key);
}
// is_signer
assert_eq!(offset_of!(AccountInfo, is_signer), 40);
let is_signer_ptr = (account_info_addr + 40) as *const bool;
unsafe {
assert!(*is_signer_ptr);
}
// is_writable
assert_eq!(offset_of!(AccountInfo, is_writable), 41);
let is_writable_ptr = (account_info_addr + 41) as *const bool;
unsafe {
assert!(!*is_writable_ptr);
}
// lamports
assert_eq!(offset_of!(AccountInfo, lamports), 8);
let lamports_ptr = (account_info_addr + 8) as *const Rc<RefCell<&mut u64>>;
unsafe {
assert_eq!(**(*lamports_ptr).as_ptr(), 31);
}
// data
assert_eq!(offset_of!(AccountInfo, data), 16);
let data_ptr = (account_info_addr + 16) as *const Rc<RefCell<&mut [u8]>>;
unsafe {
assert_eq!((*(*data_ptr).as_ptr())[..], data[..]);
}
// owner
assert_eq!(offset_of!(AccountInfo, owner), 24);
let owner_ptr = (account_info_addr + 24) as *const &Pubkey;
unsafe {
assert_eq!(**owner_ptr, owner);
}
// executable
assert_eq!(offset_of!(AccountInfo, executable), 42);
let executable_ptr = (account_info_addr + 42) as *const bool;
unsafe {
assert!(*executable_ptr);
}
// rent_epoch
assert_eq!(offset_of!(AccountInfo, rent_epoch), 32);
let renbt_epoch_ptr = (account_info_addr + 32) as *const Epoch;
unsafe {
assert_eq!(*renbt_epoch_ptr, 42);
}
}
}

For types that are used that assume a certain layout, we may need to create our own stable-layout versions of those types. Here's an example for Vec:
https://godbolt.org/z/v4qa9vGYz

I noticed at least a few types in the VM do this already with types prefixed by Sol, i.e. SolInstruction, SolAccountMeta, and SolAccountInfo:

/// Rust representation of C's SolInstruction
#[derive(Debug)]
#[repr(C)]
struct SolInstruction {
program_id_addr: u64,
accounts_addr: u64,
accounts_len: u64,
data_addr: u64,
data_len: u64,
}
/// Rust representation of C's SolAccountMeta
#[derive(Debug)]
#[repr(C)]
struct SolAccountMeta {
pubkey_addr: u64,
is_writable: bool,
is_signer: bool,
}
/// Rust representation of C's SolAccountInfo
#[derive(Debug)]
#[repr(C)]
struct SolAccountInfo {
key_addr: u64,
lamports_addr: u64,
data_len: u64,
data_addr: u64,
owner_addr: u64,
rent_epoch: u64,
#[allow(dead_code)]
is_signer: bool,
#[allow(dead_code)]
is_writable: bool,
executable: bool,
}

Proposed Solution

Audit types used by the VM that may assume layout (Instruction, AccountInfo, AccountMeta, etc) and see if any require a stable layout. Nail down those layouts and update the check_type_assumptions test.

@brooksprumo brooksprumo added the runtime Issues related to runtime, BPF, and LLVM label Jan 24, 2023
@brooksprumo
Copy link
Contributor Author

Relevant prior work: #26531

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runtime Issues related to runtime, BPF, and LLVM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant