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

Incorrect (and probably unsound) transmutes #40

Closed
niklasf opened this issue Jan 17, 2021 · 2 comments
Closed

Incorrect (and probably unsound) transmutes #40

niklasf opened this issue Jan 17, 2021 · 2 comments

Comments

@niklasf
Copy link
Contributor

@niklasf niklasf commented Jan 17, 2021

The code contains several transmutes similar to this:

#[derive(Debug, Default)]
#[cfg_attr(feature = "serialize", derive(Serialize, Deserialize))]
pub struct VendorInfo {
    ebx: u32,
    edx: u32,
    ecx: u32,
}

impl VendorInfo {
    /// Return vendor identification as human readable string.
    pub fn as_string<'a>(&'a self) -> &'a str {
        unsafe {
            let brand_string_start = self as *const VendorInfo as *const u8;
            let slice = slice::from_raw_parts(brand_string_start, 3 * 4);
            let byte_array: &'a [u8] = transmute(slice);
            str::from_utf8_unchecked(byte_array)
        }
    }
}

Reading about transmutes in the nomicon, this is not correct, because the Rust compiler is free to reorder the fields of the struct.

It is also probably unsound, because I believe the Rust compiler is technically even free to add padding.

Adding #[repr(C)] may be a fix. However I am not sure what role endianness plays here (same question as #39). is a fix. I'll submit a pull request.

niklasf added a commit to niklasf/rust-cpuid that referenced this issue Jan 17, 2021
Add comments explaining why all safety requirements are now met.
@niklasf
Copy link
Contributor Author

@niklasf niklasf commented Jan 20, 2021

Drafting a corresponding advisory here: rustsec/advisory-db#614.

@gz
Copy link
Owner

@gz gz commented Jan 20, 2021

Many thanks for fixing this!

gz added a commit that referenced this issue Jan 20, 2021
Fix unsound transmutes (fixes #40)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants