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

Potential alignment issue in AccountLoader::load() family methods #2614

Open
snawaz opened this issue Aug 23, 2023 · 1 comment
Open

Potential alignment issue in AccountLoader::load() family methods #2614

snawaz opened this issue Aug 23, 2023 · 1 comment
Labels

Comments

@snawaz
Copy link
Contributor

snawaz commented Aug 23, 2023

The type AccountLoader<'info, T> has at least 3 methods which cast acc_info.data (which is of type &[u8]) to &T.

Ok(RefMut::map(data, |data| {
bytemuck::from_bytes_mut(&mut data.deref_mut()[8..mem::size_of::<T>() + 8])
}))

It's done using bytemuck::from_bytes*() functions, which internally checks that the address of the source buffer is a multiple of the alignment of T — i.e (data.as_ptr() as usize) % align_of::<T>() == 0 has to be true!

  • But how can we be sure that such a requirement is always satisfied, in each invocation of the program?

The address where acc_info.data resides can be any address, means Solana runtime might load the account data at any address; in some invocations, the address might be a multiple of alignof<T>() (and the cast's requirement is satisfied), and in some other invocations, the address might not be a multiple of alignof<T>(), in which case we have a problem: the account cannot be loaded by any of the load*() methods (and rightly so, as there is misalignment).

Or do I miss something really important here?

As per my understanding, the load*() methods are guaranteed to work only if the alignment of T is 1 — which is the case when repr(packed) is used but not when repr(C) is used.

@acheroncrypto
Copy link
Collaborator

I think this was handled in #2330 by making it safe by default.

The address where acc_info.data resides can be any address, means Solana runtime might load the account data at any address; in some invocations, the address might be a multiple of alignof<T>() (and the cast's requirement is satisfied), and in some other invocations, the address might not be a multiple of alignof<T>(), in which case we have a problem: the account cannot be loaded by any of the load*() methods (and rightly so, as there is misalignment).

Or do I miss something really important here?

As per my understanding, the load*() methods are guaranteed to work only if the alignment of T is 1 — which is the case when repr(packed) is used but not when repr(C) is used.

They are aligned because we require zero copy accounts to be bytemuck::Pod which checks that the struct does not have any padding bytes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants