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
