[PATCH] fwts: Fix madt testing with broken MADT entries
Prarit Bhargava
prarit at redhat.com
Thu Jul 7 15:30:05 UTC 2016
Please note I am not subscribed to fwdt-devel.
P.
----8<----
When running the madt test on a system with incorrectly labelled MADTs the
following error happens:
[root at intel-wildcatpass-05 fwts]# fwts madt
Running 1 tests, results appended to results.log
Test: MADT Multiple APIC Description Table (spec compliant).
MADT checksum test. 1 passed
MADT revision test. 2 passed
MADT architecture minimum revision test. 1 passed
MADT flags field reserved bits test. 1 passed
MADT subtable tests. : 80.0% -
Caught SIGNAL 11 (Segmentation fault), aborting.
Backtrace:
0x00007f7c402ee984 /usr/local/lib/fwts/libfwts.so.1(+0xf984)
0x00007f7c3facf100 /lib64/libpthread.so.0(+0xf100)
0x00007f7c3ec3fab4 /lib64/libc.so.6(_IO_vfprintf+0x1564)
0x00007f7c3ec6b589 /lib64/libc.so.6(vsnprintf+0x79)
0x00007f7c402f3a4b /usr/local/lib/fwts/libfwts.so.1(fwts_framework_log+0xdb)
0x000000000041a156 fwts()
0x00007f7c402f352f /usr/local/lib/fwts/libfwts.so.1(+0x1452f)
0x00007f7c402f4ba3 /usr/local/lib/fwts/libfwts.so.1(fwts_framework_args+0x743)
0x0000000000404cda fwts()
0x00007f7c3ec18b15 /lib64/libc.so.6(__libc_start_main+0xf5)
0x0000000000405ad5 fwts()
This occurs because the value of hdr->type (in the above case 0x7f) can
exceed the array lengths of madt_sub_names[] and
acpi_madt_subtable_lengths->lengths[] which have a maximum index of
NUM_SUBTABLE_TYPES. This, of course, results in a segfault.
This patch implements MADT error checking for the reserved (0x10 - 0x7f)
and OEM (0x80 - 0xff) types. Any use of the reserved fields is considered
a failure, and checking of the OEM fields is restricted to moving to the
next entry as we cannot possibly know all the OEM uses of MADT entries.
This results in the test passing with new failures (as expected):
<snip>
PASSED: Test 5, MADT I/O APIC flags are reserved, and set to zero.
PASSED: Test 5, MADT I/O APIC address in non-zero.
PASSED: Test 5, MADT subtable type 127 (Reserved. OSPM skips structures of the
reserved type.) is defined.
PASSED: Test 5, Subtable 196 of type 127 (Reserved. OSPM skips structures of the
reserved type.) is the correct length: 12
FAILED [MEDIUM] SPECMADTSubReservedID: Test 5, MADT subtable 196 is using the
reserved value 0x7f for a type. Subtable type values 0x10..0x7f are reserved;
0x80..0xff can be used by OEMs.
PASSED: Test 5, MADT subtable type 127 (Reserved. OSPM skips structures of the
reserved type.) is defined.
PASSED: Test 5, Subtable 197 of type 127 (Reserved. OSPM skips structures of the
reserved type.) is the correct length: 12
FAILED [MEDIUM] SPECMADTSubReservedID: Test 5, MADT subtable 197 is using the
reserved value 0x7f for a type. Subtable type values 0x10..0x7f are reserved;
0x80..0xff can be used by OEMs.
PASSED: Test 5, MADT subtable type 2 (Interrupt Source Override) is defined.
PASSED: Test 5, Subtable 198 of type 2 (Interrupt Source Override) is the
correct length: 10
PASSED: Test 5, MADT Interrupt Source Override Bus is 0 for ISA bus.
PASSED: Test 5, MADT Interrupt Source Override flags, bits 4..31 are reserved
<snip>
and the test completing successfully:
================================================================================
1182 passed, 2 failed, 0 warning, 0 aborted, 0 skipped, 0 info only.
================================================================================
Signed-off-by: Prarit Bhargava <prarit at redhat.com>
---
src/acpi/madt/madt.c | 68 +++++++++++++++++++++++++++++++++------------
src/lib/include/fwts_acpi.h | 3 +-
2 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c
index b65b89ed4fa7..8ee231ee16be 100644
--- a/src/acpi/madt/madt.c
+++ b/src/acpi/madt/madt.c
@@ -399,6 +399,8 @@ static const char *madt_sub_names[] = {
/* 0x0d */ "GICv2m MSI Frame",
/* 0x0e */ "GICR Redistributor",
/* 0x0f */ "GIC Interrupt Translation Service (ITS)",
+ /* 0x10 - 0x7f */ "Reserved. OSPM skips structures of the reserved type.",
+ /* 0x80 - 0xff */ "Reserved for OEM use",
NULL
};
@@ -1207,8 +1209,9 @@ static int madt_subtables(fwts_framework *fw)
while (length > (ssize_t)sizeof(fwts_acpi_madt_sub_table_header)) {
ssize_t skip = 0;
- int len;
+ int len = 0;
bool passed = true;
+ int type;
hdr = (fwts_acpi_madt_sub_table_header *)data;
ii++;
@@ -1216,18 +1219,32 @@ static int madt_subtables(fwts_framework *fw)
data += sizeof(fwts_acpi_madt_sub_table_header);
length -= sizeof(fwts_acpi_madt_sub_table_header);
- /* is this subtable type defined? */
- len = ms->lengths[hdr->type];
+ /* set initial type value, will be overriden for OEM and
+ * reserved entries */
+ type = hdr->type;
+
+ /* check for OEM and reserved entries */
+ if (hdr->type >= NUM_SUBTABLE_TYPES) {
+ if (hdr->type < 0x80)
+ type = FWTS_ACPI_MADT_RESERVED;
+ else
+ type = FWTS_ACPI_MADT_OEM;
+ len = hdr->length;
+ } else {
+ /* this subtable is defined */
+ len = ms->lengths[hdr->type];
+ }
+
if (!len) {
fwts_failed(fw, LOG_LEVEL_MEDIUM,
"SPECMADTSubType",
"Undefined MADT subtable type for this "
"version of the MADT: %d (%s)",
- hdr->type, madt_sub_names[hdr->type]);
+ hdr->type, madt_sub_names[type]);
} else {
fwts_passed(fw,
"MADT subtable type %d (%s) is defined.",
- hdr->type, madt_sub_names[hdr->type]);
+ hdr->type, madt_sub_names[type]);
}
/* verify that the length is what we expect */
@@ -1250,7 +1267,7 @@ static int madt_subtables(fwts_framework *fw)
"Subtable %d of type %d (%s) is the "
" correct length: %d",
ii, hdr->type,
- madt_sub_names[hdr->type],
+ madt_sub_names[type],
hdr->length);
} else {
fwts_failed(fw, LOG_LEVEL_MEDIUM,
@@ -1258,12 +1275,12 @@ static int madt_subtables(fwts_framework *fw)
"Subtable %d of type %d (%s) is %d bytes "
" long but should be %d bytes",
ii, hdr->type,
- madt_sub_names[hdr->type],
+ madt_sub_names[type],
hdr->length, len);
}
/* perform checks specific to subtable types */
- switch (hdr->type) {
+ switch (type) {
case FWTS_ACPI_MADT_LOCAL_APIC:
skip = madt_local_apic(fw, hdr, data);
break;
@@ -1328,17 +1345,32 @@ static int madt_subtables(fwts_framework *fw)
skip = madt_gic_its(fw, hdr, data);
break;
+ case FWTS_ACPI_MADT_RESERVED:
+ fwts_failed(fw, LOG_LEVEL_MEDIUM,
+ "SPECMADTSubReservedID",
+ "MADT subtable %d is using the "
+ "reserved value 0x%x for a type. "
+ "Subtable type values 0x10..0x7f "
+ "are reserved; 0x80..0xff can be "
+ "used by OEMs.",
+ ii, hdr->type);
+ skip = (hdr->length -
+ sizeof(fwts_acpi_madt_sub_table_header));
+ break;
+ case FWTS_ACPI_MADT_OEM:
+ /* OEM entries must be assumed to be valid */
+ skip = (hdr->length -
+ sizeof(fwts_acpi_madt_sub_table_header));
+ break;
default:
- if (hdr->type >= 0x10 && hdr->type <= 0x7f)
- fwts_failed(fw, LOG_LEVEL_MEDIUM,
- "SPECMADTSubReservedID",
- "MADT subtable %d is using the "
- "reserved value 0x%x for a type. "
- "Subtable type values 0x10..0x7f "
- "are reserved; 0x80..0xff can be "
- "used by OEMs.",
- ii, hdr->type);
- skip = hdr->length;
+ fwts_failed(fw, LOG_LEVEL_MEDIUM,
+ "SPECMADTSubReservedID",
+ "MADT subtable %d is using value 0x%x "
+ "for a type. This value is out of the "
+ "expected range of 0x00 .. 0xff.",
+ ii, hdr->type);
+ skip = (hdr->length -
+ sizeof(fwts_acpi_madt_sub_table_header));
break;
}
diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h
index a3a5701c93b9..02fdceb2db11 100644
--- a/src/lib/include/fwts_acpi.h
+++ b/src/lib/include/fwts_acpi.h
@@ -488,7 +488,8 @@ typedef enum {
FWTS_ACPI_MADT_GIC_V2M_MSI_FRAME,
FWTS_ACPI_MADT_GIC_R_REDISTRIBUTOR,
FWTS_ACPI_MADT_GIC_ITS,
- FWTS_ACPI_MADT_RESERVED
+ FWTS_ACPI_MADT_RESERVED, /* does not have defined structure */
+ FWTS_ACPI_MADT_OEM /* does not have defined structure */
} fwts_acpi_madt_type;
/* Type 0, FWTS_ACPI_MADT_LOCAL_APIC */
--
1.8.3.1
More information about the fwts-devel
mailing list