[PATCH] drm/amdkfd: validate SRAT subtables before parsing

[PATCH] drm/amdkfd: validate SRAT subtables before parsing

[Jiale Yao]

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 <yaojiale02@163.com>
---
 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

Re: [PATCH] drm/amdkfd: validate SRAT subtables before parsing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/amdkfd: unaligned access and endianness bugs in SRAT parsing
--

commit 172d11efb00c0dedbe45ba294e5438f125fd1ad9
Author: Jiale Yao <yaojiale02@163.com>

drm/amdkfd: validate SRAT subtables before parsing

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. This patch validates the table header, subtable ranges, and
supported typed records to prevent out-of-bounds reads.

> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 6e0df685503d0..32d7573dffc94 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)
[ ... ]
>  		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;

[Severity: High]
cpu->proximity_domain_hi is at offset 9 in the packed acpi_srat_cpu_affinity
struct. Can casting it to a u32 pointer lead to an unaligned memory access,
potentially triggering exceptions on architectures that do not support native
unaligned loads?

Also, on Big-Endian architectures, reading proximity_domain_hi as a u32 loads
bytes in reverse order. Does shifting by 8 discard the most significant byte
instead of clock_domain[0], resulting in a corrupted pxm value?

>  			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]));

[Severity: High]
Reading gpu->device_handle using u16 pointer casts correctly extracts the
segment and BDF on Little-Endian, but can this swap the bytes of both the
segment and the BDF on Big-Endian systems? Does this break GPU NUMA node
discovery on Big-Endian platforms?

>  			if (bdf == pci_id) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016141504.2794541-1-yaojiale02@163.com?part=1

Re:[PATCH] drm/amdkfd: validate SRAT subtables before parsing

[jiale yao]

At 2026-10-16 22:15:04, "Jiale Yao" <yaojiale02@163.com> wrote:
>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.
>

Fixes: ddec8d3be0f8 ("drm/amdkfd: add ACPI SRAT parsing for topology")

>Signed-off-by: Jiale Yao <yaojiale02@163.com>
>---
> 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

Re: [PATCH] drm/amdkfd: validate SRAT subtables before parsing

[Alex Deucher]

On Sat, Oct 17, 2026 at 2:26 PM Jiale Yao <yaojiale02@163.com> wrote:
>
> 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 <yaojiale02@163.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
and applied.  Thanks!

> ---
>  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
>