Hi,
you are right that i have to do some reading regarding the ACPI spec.
But i did some fast search and actually thought C4 was a possibility.
Looking into the ACPI spec now (chapter 8.1 Processor Power States) i
find the following: "... Processor power states include are designated
C0, C1, C2, C3, . . . Cn...". And later: "...The C1 through Cn power
states are,processor sleeping states...". And chapter 8.1.5 is about
power states beyound C3. But i guess it is reasonable to skip them for now.
Regarding your patch i think it would be a good idea to use a define
like 'CSTATE_MAX' for the maximum supported C state and use this define
inside the array definition and the condition inside
acpicpu_add_cstatepkg. Something like:
unsigned long cst_stats[CSTATE_MAX + 1]
...
if (state < ACPI_STATE_C1 || state > CSTATE_MAX)
Kind regards,
Stefan
Am 01.11.26 um 05:23 schrieb Philip Guenther:
Thank you. You've identified a problem, but I think it actually goes the other
direction: the test inside acpicpu_add_cstatepkg() is wrong. ACPI only defines C0 to
C3, so the test should be to reject state > 3, not state > 4. Indeed, it
should also be rejecting a state of 0, as the ACPI spec explicitly says that C0 is
not a valid idle Cstate package state value.
So yes, right now buggy or malicious ACPI tables could overrun the array. Of
course, if an attacker could control your ACPI tables they could trivially take
over the system via other means. Being more robust against the (many) buggy
ACPI tables out there is a worthy thing, so I think we should do this:
--- sys/dev/acpi/acpicpu_x86.c 16 Sep 2025 12:18:10 -0000 1.2
+++ sys/dev/acpi/acpicpu_x86.c 1 Nov 2026 04:20:10 -0000
@@ -382,7 +382,7 @@ acpicpu_add_cstatepkg(struct aml_value *
/* range and sanity checks */
state = val->v_package[1]->v_integer;
- if (state < 0 || state > 4)
+ if (state < 1 || state > 3)
return;
if (val->v_package[0]->type != AML_OBJTYPE_BUFFER) {
printf(": C%d (unexpected ACPI object type %d)",
The discussion of idle states in the (freely downloadable) ACPI spec has useful
information if you're considering working on that code and I recommend
reviewing it for the broader discussion.
Philip Guenther
________________________________
From: [email protected] <[email protected]> on behalf of Stefan Hahne
<[email protected]>
Sent: Saturday, October 31, 2026 11:02 AM
To: [email protected] <[email protected]>
Subject: Probably an off by one in acpicpu_x86.c
Hi, i'm currently looking into the idle behavior of OpenBSD and try to add
measurement variables inside the kernel that i can evaluate using ddb. Doing
this i looked into acpicpu_x86. c. I think the cst_stats array is one element
too small as
Hi,
i'm currently looking into the idle behavior of OpenBSD and try to add
measurement variables inside the kernel that i can evaluate using ddb.
Doing this i looked into acpicpu_x86.c. I think the cst_stats array is
one element too small as the check inside the function
acpicpu_add_cstatepkg allows an index up to 4.
Kind regards,
Stefan
