[PATCH] PCI/ACPI: validate _HPX type 3 record lengths

[PATCH] PCI/ACPI: validate _HPX type 3 record lengths

[Kang Chen]

program_type3_hpx_record() computes the expected record length as
3 + desc_count * 14 in 32 bits.  A firmware-provided desc_count of
0x80000000 makes this wrap to 3, so the package count check passes for
a three-element record and the loop then walks far beyond the elements
array, feeding out-of-bounds data into PCI config space writes.  The
record is also indexed at fields[1] and fields[2] before its length has
been checked.

Reject records with fewer than three elements, verify that fields[1]
and fields[2] are integers, and reject desc_count values for which the
expected length would overflow.  Reject revision or descriptor count
values that do not fit in 32 bits instead of silently truncating them.
Use an unsigned loop index and advance the register pointer explicitly
so the walk cannot overflow either.  In acpi_run_hpx(), reject records
that do not have at least the two elements needed to read the type and
revision.

Fixes: f873c51a155a ("PCI/ACPI: Implement _HPX Type 3 Setting Record")
Signed-off-by: Kang Chen <k.chen@smail.nju.edu.cn>
---
 drivers/pci/pci-acpi.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 1150f2fbabf4..b15700523810 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -607,12 +607,28 @@ static acpi_status program_type3_hpx_record(struct pci_dev *dev,
 	u32 desc_count, expected_length, revision;
 	union acpi_object *reg_fields;
 	struct hpx_type3 hpx3;
-	int i;
+	u32 i;
+
+	if (record->package.count < 3)
+		return AE_ERROR;
+
+	if (fields[1].type != ACPI_TYPE_INTEGER ||
+	    fields[2].type != ACPI_TYPE_INTEGER)
+		return AE_ERROR;
+
+	if (fields[1].integer.value > U32_MAX)
+		return AE_ERROR;
 
 	revision = fields[1].integer.value;
 	switch (revision) {
 	case 1:
+		if (fields[2].integer.value > U32_MAX)
+			return AE_ERROR;
+
 		desc_count = fields[2].integer.value;
+		if (desc_count > (UINT_MAX - 3) / 14)
+			return AE_ERROR;
+
 		expected_length = 3 + desc_count * 14;
 
 		if (record->package.count != expected_length)
@@ -622,10 +638,11 @@ static acpi_status program_type3_hpx_record(struct pci_dev *dev,
 			if (fields[i].type != ACPI_TYPE_INTEGER)
 				return AE_ERROR;
 
+		reg_fields = fields + 3;
 		for (i = 0; i < desc_count; i++) {
-			reg_fields = fields + 3 + i * 14;
 			parse_hpx3_register(&hpx3, reg_fields);
 			program_hpx_type3(dev, &hpx3);
+			reg_fields += 14;
 		}
 
 		break;
@@ -661,7 +678,8 @@ static acpi_status acpi_run_hpx(struct pci_dev *dev, acpi_handle handle)
 
 	for (i = 0; i < package->package.count; i++) {
 		record = &package->package.elements[i];
-		if (record->type != ACPI_TYPE_PACKAGE) {
+		if (record->type != ACPI_TYPE_PACKAGE ||
+		    record->package.count < 2) {
 			status = AE_ERROR;
 			goto exit;
 		}
-- 
2.34.1

Re: [PATCH] PCI/ACPI: validate _HPX type 3 record lengths

[sashiko-bot]

> program_type3_hpx_record() computes the expected record length as
> 3 + desc_count * 14 in 32 bits.  A firmware-provided desc_count of
> 0x80000000 makes this wrap to 3, so the package count check passes for
> a three-element record and the loop then walks far beyond the elements
> array, feeding out-of-bounds data into PCI config space writes.  The
> record is also indexed at fields[1] and fields[2] before its length has
> been checked.
> 
> Reject records with fewer than three elements, verify that fields[1]
> and fields[2] are integers, and reject desc_count values for which the
> expected length would overflow.  Reject revision or descriptor count
> values that do not fit in 32 bits instead of silently truncating them.
> Use an unsigned loop index and advance the register pointer explicitly
> so the walk cannot overflow either.  In acpi_run_hpx(), reject records
> that do not have at least the two elements needed to read the type and
> revision.
> 
> Fixes: f873c51a155a ("PCI/ACPI: Implement _HPX Type 3 Setting Record")
> Signed-off-by: Kang Chen <k.chen@smail.nju.edu.cn>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103114112.74527-1-k.chen@smail.nju.edu.cn?part=1