Add upper limit for PCI capabilities

Prevent loops on malformed PCI trees
F/232

Author: danielinux

src/boot_x86_fsp.c

@@ -82,6 +82,9 @@ const uint8_t __attribute__((section(".sig_wolfboot_raw")))
 #define FSP_STATUS_RESET_REQUIRED_WARM  0x40000002
 #define MEMORY_4GB (4ULL * 1024 * 1024 * 1024)
 #define ENDLINE "\r\n"
+/* Standard PCI capabilities live in conventional config space at 0x40-0xFC,
+ * on 4-byte alignment, so there are at most 48 distinct capability headers. */
+#define PCI_MAX_STANDARD_CAPABILITIES 48
 
 
 /* compile time alignment checks */
@@ -342,19 +345,21 @@ static int pci_get_capability(uint8_t bus, uint8_t dev, uint8_t fun,
                               uint8_t cap_id, uint8_t *cap_off)
 {
     uint8_t r8, id;
+    uint8_t cap_count = 0;
     uint32_t r32;
 
     r32 = pci_config_read16(bus, dev, fun, PCI_STATUS_OFFSET);
     if (!(r32 & PCI_STATUS_CAP_LIST))
         return -1;
     r8 = pci_config_read8(bus, dev, fun, PCI_CAP_OFFSET);
-    while (r8 != 0) {
+    while ((r8 != 0) && (cap_count < PCI_MAX_STANDARD_CAPABILITIES)) {
         id = pci_config_read8(bus, dev, fun, r8);
         if (id == cap_id) {
             *cap_off = r8;
             return 0;
         }
         r8 = pci_config_read8(bus, dev, fun, r8 + 1);
+        cap_count++;
     }
     return -1;
 }

tools/unit-tests/Makefile

@@ -38,7 +38,7 @@ TESTS:=unit-parser unit-extflash unit-string unit-spi-flash unit-aes128 \
        unit-image unit-image-rsa unit-nvm unit-nvm-flagshome unit-enc-nvm \
        unit-enc-nvm-flagshome unit-delta unit-update-flash \
        unit-update-flash-enc unit-update-ram unit-pkcs11_store unit-psa_store unit-disk \
-       unit-multiboot
+       unit-multiboot unit-boot-x86-fsp
 
 all: $(TESTS)
 
@@ -121,6 +121,11 @@ unit-chacha20: ../../include/target.h unit-extflash.c
 unit-pci:  unit-pci.c ../../src/pci.c
 	gcc -o $@ $< $(CFLAGS) -DWOLFBOOT_USE_PCI $(LDFLAGS)
 
+unit-boot-x86-fsp: ../../include/target.h unit-boot-x86_fsp.c
+	gcc -o $@ $^ $(CFLAGS) -DWOLFBOOT_LOAD_BASE=0x100000 -DWOLFBOOT_FSP \
+		-DUCODE0_ADDRESS=0 -ffunction-sections -fdata-sections $(LDFLAGS) \
+		-Wl,--gc-sections
+
 unit-mock-state: ../../include/target.h unit-mock-state.c
 	gcc -o $@ $^ $(CFLAGS) $(LDFLAGS)
 

tools/unit-tests/unit-boot-x86_fsp.c

@@ -0,0 +1,111 @@
+/* unit-boot-x86_fsp.c
+ *
+ * Unit tests for selected boot_x86_fsp helpers.
+ */
+
+#include <check.h>
+#include <stdint.h>
+#include <string.h>
+
+#define TEST_CFG_SIZE 256
+#define TEST_READ_LIMIT 200
+
+static uint8_t test_cfg[TEST_CFG_SIZE];
+static unsigned int test_read_count;
+
+uint8_t pci_config_read8(uint8_t bus, uint8_t dev, uint8_t fun, uint8_t off)
+{
+    (void)bus;
+    (void)dev;
+    (void)fun;
+
+    test_read_count++;
+    ck_assert_msg(test_read_count < TEST_READ_LIMIT,
+                  "pci_get_capability exceeded read limit on cyclic list");
+    return test_cfg[off];
+}
+
+uint16_t pci_config_read16(uint8_t bus, uint8_t dev, uint8_t fun, uint8_t off)
+{
+    uint16_t value;
+
+    (void)bus;
+    (void)dev;
+    (void)fun;
+
+    test_read_count++;
+    ck_assert_msg(test_read_count < TEST_READ_LIMIT,
+                  "pci_get_capability exceeded read limit on cyclic list");
+    memcpy(&value, &test_cfg[off], sizeof(value));
+    return value;
+}
+
+#include "../../src/boot_x86_fsp.c"
+
+static void setup(void)
+{
+    memset(test_cfg, 0, sizeof(test_cfg));
+    test_read_count = 0;
+}
+
+START_TEST(test_pci_get_capability_finds_requested_capability)
+{
+    uint8_t cap_off = 0;
+    uint16_t status = PCI_STATUS_CAP_LIST;
+
+    memcpy(&test_cfg[PCI_STATUS_OFFSET], &status, sizeof(status));
+    test_cfg[PCI_CAP_OFFSET] = 0x40;
+    test_cfg[0x40] = 0x01;
+    test_cfg[0x41] = 0x48;
+    test_cfg[0x48] = PCI_PCIE_CAP_ID;
+    test_cfg[0x49] = 0x00;
+
+    ck_assert_int_eq(pci_get_capability(0, 0, 0, PCI_PCIE_CAP_ID, &cap_off), 0);
+    ck_assert_uint_eq(cap_off, 0x48);
+}
+END_TEST
+
+START_TEST(test_pci_get_capability_rejects_cyclic_capability_lists)
+{
+    uint8_t cap_off = 0xAA;
+    uint16_t status = PCI_STATUS_CAP_LIST;
+
+    memcpy(&test_cfg[PCI_STATUS_OFFSET], &status, sizeof(status));
+    test_cfg[PCI_CAP_OFFSET] = 0x40;
+    test_cfg[0x40] = 0x01;
+    test_cfg[0x41] = 0x48;
+    test_cfg[0x48] = 0x05;
+    test_cfg[0x49] = 0x40;
+
+    ck_assert_int_eq(pci_get_capability(0, 0, 0, PCI_PCIE_CAP_ID, &cap_off), -1);
+    ck_assert_uint_eq(cap_off, 0xAA);
+}
+END_TEST
+
+static Suite *boot_x86_fsp_suite(void)
+{
+    Suite *s;
+    TCase *tc;
+
+    s = suite_create("boot_x86_fsp");
+    tc = tcase_create("pci_get_capability");
+    tcase_add_checked_fixture(tc, setup, NULL);
+    tcase_add_test(tc, test_pci_get_capability_finds_requested_capability);
+    tcase_add_test(tc, test_pci_get_capability_rejects_cyclic_capability_lists);
+    suite_add_tcase(s, tc);
+    return s;
+}
+
+int main(void)
+{
+    Suite *s;
+    SRunner *sr;
+    int failed;
+
+    s = boot_x86_fsp_suite();
+    sr = srunner_create(s);
+    srunner_run_all(sr, CK_NORMAL);
+    failed = srunner_ntests_failed(sr);
+    srunner_free(sr);
+    return failed == 0 ? 0 : 1;
+}