[PATCH] acpi: madt: Fix processor UID check

Jeffrey Hugo jhugo at codeaurora.org
Fri Oct 14 16:35:22 UTC 2016


Commit 763737164714 ("fwts/madt: Add processor UID checking to madt tests")
is broken on the Qualcomm Technologies QDF2432 and likely very fragile on
other platforms as well.

The root cause is that the added redefinition of the acpi_integer structure
is not compatible with AcpiEvaluateObject() and can result in
AE_BUFFER_OVERFLOW errors.

AcpiEvaluateObject() ensures that the provided return buffer is large enough
to hold the resulting object.  The size of the resulting object is
determined via AcpiUtGetSimpleObjectSize() which uses ACPI_OBJECT as a
baseline for the size of a resulting object (variable length objects like
strings may be larger).  Per src/acpica/source/include/actypes.h ACPI_OBJECT
is a union, and therefore the size of ACPI_OBJECT is atleast the size of its
largest member.  The Integer member is not the largest member, thus an
ACPI_OBJECT cannot fit into acpi_integer struct as defined in the madt test.
On QDF2432, sizeof(acpi_integer) is 16, where as sizeof(ACPI_OBJECT) is 24.
Since 24 > 16, AcpiEvaluateObject returns AE_BUFFER_OVERFLOW.

This results in false test failures because the test has not properly parsed
the processor device UIDs, and cannot later match the GICC UIDs to any
processor UIDs.

We fix this by not reinventing the wheel, and using ACPI_OBJECT as our
output buffer for AcpiEvaluateObject (both Processor and Integer objects
that we care about are fixed size), and parsing the union nativly for the
values we care about.  Since the source object is also ACPI_OBJECT, the
output buffer will always be of the same type, and thus the same size,
preventing AE_BUFFER_OVERFLOW errors, particularly if ACPI_OBJECT grows in
size in the future (ie an added member, or some stange compiler packing).

Signed-off-by: Jeffrey Hugo <jhugo at codeaurora.org>
---

I'm not entirely sure how this worked in the first place, but since Prarit
origionally wrote this check, I'd appreciate if Prarit would test this fix
to verify it doesn't cause a regression in their usecase.

 src/acpi/madt/madt.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c
index b087d27..6c7dee2 100644
--- a/src/acpi/madt/madt.c
+++ b/src/acpi/madt/madt.c
@@ -232,13 +232,6 @@ static fwts_list msi_frame_ids;
 static fwts_list its_ids;
 static fwts_list processor_uids;
 
-struct acpi_processor {
-	ACPI_OBJECT_TYPE type;
-	uint32_t proc_id;
-	ACPI_IO_ADDRESS pblk_address;
-	uint32_t pblk_length;
-};
-
 struct acpi_integer {
 	ACPI_OBJECT_TYPE type;
 	uint64_t value;
@@ -249,10 +242,8 @@ static ACPI_STATUS madt_processor_handler(ACPI_HANDLE ObjHandle, uint32_t level,
 {
 	ACPI_OBJECT_TYPE acpi_type;
 	ACPI_STATUS status;
-	struct acpi_processor processor;
-	struct acpi_integer integer;
-	struct acpi_buffer pbuf = {sizeof(struct acpi_processor), &processor};
-	struct acpi_buffer ibuf = {sizeof(struct acpi_integer), &integer};
+	ACPI_OBJECT obj;
+	struct acpi_buffer buf = {sizeof(ACPI_OBJECT), &obj};
 	struct acpi_integer *listint;
 
 	/* Prevent -Werror=unused-parameter from complaining */
@@ -272,20 +263,20 @@ static ACPI_STATUS madt_processor_handler(ACPI_HANDLE ObjHandle, uint32_t level,
 
 	switch(acpi_type) {
 	case ACPI_TYPE_PROCESSOR:
-		status = AcpiEvaluateObject(ObjHandle, NULL, NULL, &pbuf);
+		status = AcpiEvaluateObject(ObjHandle, NULL, NULL, &buf);
 		if (ACPI_FAILURE(status)) {
 			free(listint);
 			return status;
 		}
-		listint->value = processor.proc_id;
+		listint->value = obj.Processor.ProcId;
 		break;
 	case ACPI_TYPE_DEVICE:
-		status = AcpiEvaluateObject(ObjHandle, "_UID", NULL, &ibuf);
+		status = AcpiEvaluateObject(ObjHandle, "_UID", NULL, &buf);
 		if (ACPI_FAILURE(status)) {
 			free(listint);
 			return status;
 		}
-		listint->value = integer.value;
+		listint->value = obj.Integer.Value;
 		break;
 	default:
 		free(listint);
-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.




More information about the fwts-devel mailing list