Skip to content

fix(rsdp): fix rsdp revision and rsdp checksum to fit linux kernel impl#319

Closed
donjuanplatinum wants to merge 2 commits into
rust-osdev:mainfrom
donjuanplatinum:rsdp-fix
Closed

fix(rsdp): fix rsdp revision and rsdp checksum to fit linux kernel impl#319
donjuanplatinum wants to merge 2 commits into
rust-osdev:mainfrom
donjuanplatinum:rsdp-fix

Conversation

@donjuanplatinum

Copy link
Copy Markdown
Contributor
  1. In Linux kernel source in drivers/acpi/acpica/tbutils.c line 242-259 shows that: when revision > 1, use XSDT. and revision = 0 or 1 use RSDT.
	/* Use XSDT if present and not overridden. Otherwise, use RSDT */

	if ((rsdp->revision > 1) &&
	    rsdp->xsdt_physical_address && !acpi_gbl_do_not_use_xsdt) {
		/*
		 * RSDP contains an XSDT (64-bit physical addresses). We must use
		 * the XSDT if the revision is > 1 and the XSDT pointer is present,
		 * as per the ACPI specification.
		 */
		address = (acpi_physical_address)rsdp->xsdt_physical_address;
		table_entry_size = ACPI_XSDT_ENTRY_SIZE;
	} else {
		/* Root table is an RSDT (32-bit physical addresses) */

		address = (acpi_physical_address)rsdp->rsdt_physical_address;
		table_entry_size = ACPI_RSDT_ENTRY_SIZE;
	}

in acpi-rs in acpi/src/lib.rs line 233-248 shows that: only revision ==0 use RSDT, other use XSDT.

 let revision = rsdp_mapping.revision();
        let root_table_mapping = if revision == 0 {
            /*
             * We're running on ACPI Version 1.0. We should use the 32-bit RSDT address.
             */

            read_root_table!(RSDT, rsdt_address)
        } else {
            /*
             * We're running on ACPI Version 2.0+. We should use the 64-bit XSDT address, truncated
             * to 32 bits on x86.
             */

            read_root_table!(XSDT, xsdt_address)
        };

        Ok(Self { mapping: root_table_mapping, revision, handler })
    }

so , when revision = 1, linux will regard it as RSDT, but acpi-rs see it as a XSDT.
2. the checksum len in acpi 2.0 condition.
in linux source drivers/acpi/acpica/tbxfroot.c line 59-89.

the acpi2.0 checksum len is a constant ACPI_RSDP_XCHECKSUM_LENGTH valued 36 and its defined in include/acpi/acconfig.h in line 173:

/* RSDP checksums */
#define ACPI_RSDP_CHECKSUM_LENGTH       20
#define ACPI_RSDP_XCHECKSUM_LENGTH      36
acpi_status acpi_tb_validate_rsdp(struct acpi_table_rsdp *rsdp)
{

	/*
	 * The signature and checksum must both be correct
	 *
	 * Note: Sometimes there exists more than one RSDP in memory; the valid
	 * RSDP has a valid checksum, all others have an invalid checksum.
	 */
	if (!ACPI_VALIDATE_RSDP_SIG(rsdp->signature)) {

		/* Nope, BAD Signature */

		return (AE_BAD_SIGNATURE);
	}

	/* Check the standard checksum */

	if (acpi_ut_checksum((u8 *)rsdp, ACPI_RSDP_CHECKSUM_LENGTH) != 0) {
		return (AE_BAD_CHECKSUM);
	}

	/* Check extended checksum if table version >= 2 */

	if ((rsdp->revision >= 2) &&
	    (acpi_ut_checksum((u8 *)rsdp, ACPI_RSDP_XCHECKSUM_LENGTH) != 0)) {
		return (AE_BAD_CHECKSUM);
	}

	return (AE_OK);
}

but in acpi-rs in acpi/src/rsdp.rs line 119-127 it use self.length

   let length = if self.revision > 0 {
            // For Version 2.0+, include the number of bytes specified by `length`
            self.length as usize
        } else {
            RSDP_V1_LENGTH
        };

        let bytes = unsafe { slice::from_raw_parts(self as *const Rsdp as *const u8, length) };
        let sum = bytes.iter().fold(0u8, |sum, &byte| sum.wrapping_add(byte));

so when the length of RSDP in memory is not 36 and use ACPI2.0, linux will also see it as 36,but acpi-rs not.

and if firmware write the lenght > 36, acpi-rs will read over the range and go into UB.

donjuanplatinum and others added 2 commits July 16, 2026 14:07
Signed-off-by: Donjuanplatinum <donplat@barrensea.org>
IsaacWoods added a commit to IsaacWoods/acpi that referenced this pull request Jul 16, 2026
…haviour

Closes rust-osdev#319

This fixes a misinterpretation of the RSDP revision and whether the address
of the XSDT or RSDT should be used, which the spec defines fairly poorly.
It cleans up downstream interpretation of which table is being used by
defining the actual entry size rather than relying on interpretation of the
RSDP revision. It also introduces an `AcpiQuirks` structure, with a first
quirk to override this behaviour and use the RSDT regardless of RSDP revision.
This does introduce breaking changes to public API, and will require a major
version bump.
@IsaacWoods

Copy link
Copy Markdown
Member

Thanks for the report! This is a good point (and another place the spec falls short).

I hope you don't mind, but I wanted to approach this in a slightly different way than your PR, so I've made a couple of commits to main with you as co-author. These implement both of your fixes, but with an additional fix (your version does not correctly infer the RSDT/XSDT entry size from the revision later, so I've moved to inferring it at the start), and adds an AcpiQuirks structure I've been meaning to chuck in for a while.

These do technically break public API, so will need a major version bump. We've got some more changes with the same requirement coming soon, so I won't do this quite yet.

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 this pull request may close these issues.

2 participants