The KFD SRAT walker reads the first subtable length without checking
that a header is present and trusts subsequent lengths when accessing
CPU and generic affinity records. Invalid firmware lengths can therefore
cause out-of-bounds reads.

Validate the table header, every generic subtable range, and each
supported typed record before reading its fields.

Signed-off-by: Jiale Yao <[email protected]>
---
 drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 35 ++++++++++++++++++---------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index 6e0df685503d..32d7573dffc9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -2056,27 +2056,33 @@ static void kfd_find_numa_node_in_srat(struct kfd_node 
*kdev)
                return;
        }
 
+       if (table_header->length < sizeof(struct acpi_table_srat))
+               goto invalid_srat;
+
        table_end = (unsigned long)table_header + table_header->length;
 
        /* Parse all entries looking for a match. */
        sub_header = (struct acpi_subtable_header *)
                        ((unsigned long)table_header +
                        sizeof(struct acpi_table_srat));
-       subtable_len = sub_header->length;
 
-       while (((unsigned long)sub_header) + subtable_len  < table_end) {
-               /*
-                * If length is 0, break from this loop to avoid
-                * infinite loop.
-                */
-               if (subtable_len == 0) {
-                       pr_err("SRAT invalid zero length\n");
-                       break;
-               }
+       while ((unsigned long)sub_header < table_end) {
+               unsigned long start = (unsigned long)sub_header;
+
+               if (table_end - start < sizeof(*sub_header))
+                       goto invalid_srat;
+
+               subtable_len = sub_header->length;
+               if (subtable_len < sizeof(*sub_header) ||
+                   subtable_len > table_end - start)
+                       goto invalid_srat;
 
                switch (sub_header->type) {
                case ACPI_SRAT_TYPE_CPU_AFFINITY:
                        cpu = (struct acpi_srat_cpu_affinity *)sub_header;
+                       if (subtable_len < sizeof(*cpu))
+                               goto invalid_srat;
+
                        pxm = *((u32 *)cpu->proximity_domain_hi) << 8 |
                                        cpu->proximity_domain_lo;
                        if (pxm > max_pxm)
@@ -2084,6 +2090,9 @@ static void kfd_find_numa_node_in_srat(struct kfd_node 
*kdev)
                        break;
                case ACPI_SRAT_TYPE_GENERIC_AFFINITY:
                        gpu = (struct acpi_srat_generic_affinity *)sub_header;
+                       if (subtable_len < sizeof(*gpu))
+                               goto invalid_srat;
+
                        bdf = *((u16 *)(&gpu->device_handle[0])) << 16 |
                                        *((u16 *)(&gpu->device_handle[2]));
                        if (bdf == pci_id) {
@@ -2100,9 +2109,13 @@ static void kfd_find_numa_node_in_srat(struct kfd_node 
*kdev)
 
                sub_header = (struct acpi_subtable_header *)
                                ((unsigned long)sub_header + subtable_len);
-               subtable_len = sub_header->length;
        }
 
+       goto out;
+
+invalid_srat:
+       pr_err("SRAT contains an invalid subtable\n");
+out:
        acpi_put_table(table_header);
 
        /* Workaround bad cpu-gpu binding case */
-- 
2.34.1

Reply via email to