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

Make pointer::align_offset exploit allocation alignment #873

Closed
RalfJung opened this issue Aug 2, 2019 · 4 comments · Fixed by #1023
Closed

Make pointer::align_offset exploit allocation alignment #873

RalfJung opened this issue Aug 2, 2019 · 4 comments · Fixed by #1023
Labels
A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement

Comments

@RalfJung
Copy link
Member

RalfJung commented Aug 2, 2019

Miri's implementation of align_offset currently always says "nope, can't do that". We could do better than that in case the alignment of the allocation of the argument pointer is at least the requested alignment. In that case, we can basically run the real implementation of align_offset from libcore.

Testcase by @shepmaster:

use std::mem;

#[derive(Debug, Default)]
#[repr(align(8))]
struct AlignToU64<T>(T);

const BYTE_LEN: usize = mem::size_of::<[u64; 4]>();
type Data = AlignToU64<[u8; BYTE_LEN]>;

fn example(data: &Data) {
    let (head, u64_arrays, tail) = unsafe { data.0.align_to::<[u64; 4]>() };

    assert!(head.is_empty(), "buffer was not aligned for 64-bit numbers");
    assert_eq!(u64_arrays.len(), 1, "buffer was not long enough");
    assert!(tail.is_empty(), "buffer was too long");

    let u64_array = &u64_arrays[0];
    let _val = u64_array[0]; // make sure we can actually load this
}

fn main() {
    example(&Data::default());
}

There should also be another testcase where the modulo stuff becomes more interesting (align_offset returns an offset in units of size_of::<T>(), not bytes).

@RalfJung RalfJung added C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement A-shims Area: This affects the external function shims labels Aug 2, 2019
@RalfJung
Copy link
Member Author

RalfJung commented Aug 2, 2019

One potential complication here is that this might make code run SSE where it did not before, because the aligned part is using SSE instructions. We should add test cases at least for libstd uses for align_to that, with this change, execute the inner loops -- to see if that runs anything Miri doesn't like (or catches any bugs).

@RalfJung
Copy link
Member Author

Also see rust-lang/rust#62420 for a detailed discussion about the benefits of making align_offset fail even though it could succeed.

bors added a commit that referenced this issue Sep 16, 2019
Use libcore's align_offset

Related issue: #873
@pvdrz
Copy link
Contributor

pvdrz commented Oct 24, 2019

I think this was solved (at least partially) by #945

@RalfJung
Copy link
Member Author

Yes this is solved; the only thing that's missing is adding the testcase to our test suite. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants