Skip to content

Invalid safety assumption in SdtHeaderIterator #245

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

Open
usadson opened this issue Mar 7, 2025 · 0 comments · May be fixed by #246
Open

Invalid safety assumption in SdtHeaderIterator #245

usadson opened this issue Mar 7, 2025 · 0 comments · May be fixed by #246

Comments

@usadson
Copy link

usadson commented Mar 7, 2025

The SdtHeaderIterator can be used to iterate over every SdtHeader, but the iterator only maps the size of the SdtHeader (36 bytes), whilst the validation requires that the whole header is mapped (for SSDTs this can be quite large, spanning over multiple pages).

This can be seen in acpi/src/lib.rs:503:

impl<H> Iterator for SdtHeaderIterator<'_, H>
where
    H: AcpiHandler,
{
    type Item = SdtHeader;

    fn next(&mut self) -> Option<Self::Item> {
        // ...

        // SAFETY: `address` needs to be valid for the size of `SdtHeader`, or the ACPI tables are malformed (not a
        // software issue).
        let header_mapping = unsafe {
            self.handler.map_physical_region::<SdtHeader>(table_phys_ptr as usize, mem::size_of::<SdtHeader>())
        };
        let r = header_mapping.validate(header_mapping.signature);

        // ...
    }
}

The last line calls SdtHeader::validate()

pub fn validate(&self, signature: Signature) -> AcpiResult<()> {
    self.validate_header_fields(signature)?;
    self.validate_checksum(signature)?;

    Ok(())
}

Which in turn calls SdtHeader::validate_checksum():

fn validate_checksum(&self, signature: Signature) -> AcpiResult<()> {
        // SAFETY: Entire table is mapped.
        let table_bytes =
            unsafe { core::slice::from_raw_parts((self as *const SdtHeader).cast::<u8>(), self.length as usize) };
        // ...
}

The SAFETY-assumption is that the entire table is mapped, whilst only the header was mapped.

@IsaacWoods IsaacWoods linked a pull request Mar 25, 2025 that will close this issue
26 tasks
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

Successfully merging a pull request may close this issue.

1 participant