[PATCH v2 0/5] ACPI: NFIT: harden subtable extent validation

[PATCH v2 0/5] ACPI: NFIT: harden subtable extent validation

[Pengpeng Hou]

NFIT parsing has several independent extent contracts: the outer table
must contain each advertised subtable, and variable or versioned records
must fit inside that subtable.  The original patch mixed those contracts
and also derived the SPA structure length from the location-cookie-valid
flag.

Split the checks by record type.  Patch 1 validates the generic subtable
header and outer extent.  Patch 2 accepts both defined SPA layouts while
treating the cookie flag as a validity bit, not a size selector.  Patches
3 and 4 validate the variable arrays and the control-region extent.
Patch 5 bounds the 32-bit capability mask without rejecting future
firmware that advertises a higher capability index.

Changes since v1:
https://lore.kernel.org/all/20260722041701.21078-1-pengpeng@iscas.ac.cn/
- split the generic and type-specific contracts into separate patches
- handle both 56-byte and 64-byte SPA structures
- add the control-region extent check identified during review
- clamp the implemented capability mask instead of rejecting future bits
- describe the exact fields protected by each check

The series was reviewed statically against the current tree. I did not
run an NFIT firmware fault-injection test.

Pengpeng Hou (5):
  ACPI: NFIT: validate subtable extents before dispatch
  ACPI: NFIT: accept SPA structures with an unused location cookie
  ACPI: NFIT: validate interleave and flush array extents
  ACPI: NFIT: validate control region extent
  ACPI: NFIT: bound the platform capability mask

 drivers/acpi/nfit/core.c | 77 ++++++++++++++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 15 deletions(-)

base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a

-- 
2.50.1 (Apple Git-155)

[PATCH v2 1/5] ACPI: NFIT: validate subtable extents before dispatch

[Pengpeng Hou]

add_table() checks only that the subtable cursor is before the end of
the NFIT before reading the two-field subtable header. A cursor with
fewer than sizeof(struct acpi_nfit_header) bytes remaining therefore
makes the header read cross the mapped table.

The function also advances by the firmware-provided length without
checking that the advertised subtable fits in the enclosing NFIT. A
malformed length can move the parser beyond the table and make the next
iteration read unrelated memory.

Require a complete header and bind the advertised length to both the
header size and the bytes remaining in the NFIT before dispatching the
subtable.

Fixes: b94d5230d06e ("libnvdimm, nfit: initial libnvdimm infrastructure and NFIT support")
Assisted-by: Codex:gpt-5
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/acpi/nfit/core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index cb771d9cadb2..4428adb6a1ab 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -963,14 +963,18 @@ static void *add_table(struct acpi_nfit_desc *acpi_desc,
 	struct device *dev = acpi_desc->dev;
 	struct acpi_nfit_header *hdr;
 	void *err = ERR_PTR(-ENOMEM);
+	size_t table_len;
 
 	if (table >= end)
 		return NULL;
+	table_len = end - table;
+	if (table_len < sizeof(*hdr))
+		return NULL;
 
 	hdr = table;
-	if (!hdr->length) {
-		dev_warn(dev, "found a zero length table '%d' parsing nfit\n",
-			hdr->type);
+	if (hdr->length < sizeof(*hdr) || hdr->length > table_len) {
+		dev_warn(dev, "invalid table length %u for type %u parsing nfit\n",
+			 hdr->length, hdr->type);
 		return NULL;
 	}
 
-- 
2.50.1 (Apple Git-155)

[PATCH v2 2/5] ACPI: NFIT: accept SPA structures with an unused location cookie

[Pengpeng Hou]

ACPI 6.4 extended the System Physical Address Range Structure from 56
to 64 bytes by appending an eight-byte location cookie. The cookie-valid
flag describes whether that field contains usable data; it does not
select the structure length.

sizeof_spa() instead derives the expected length from the flag. It
therefore rejects a valid 64-byte ACPI 6.4 structure when the cookie is
present but not valid. It can also compare 64 bytes against a previously
saved 56-byte structure without first checking that the saved allocation
has the same length.

Accept either the legacy 56-byte layout or the ACPI 6.4 64-byte layout.
Require the cookie-valid flag to be clear for the legacy layout, and
compare saved entries only when their validated lengths match.

Fixes: e9cfd259c6d3 ("ACPI: NFIT: Fix support for variable 'SPA' structure size")
Assisted-by: Codex:gpt-5
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/acpi/nfit/core.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 4428adb6a1ab..f68edfe64952 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -705,9 +705,20 @@ int nfit_spa_type(struct acpi_nfit_system_address *spa)
 
 static size_t sizeof_spa(struct acpi_nfit_system_address *spa)
 {
+	size_t legacy_size = offsetof(struct acpi_nfit_system_address,
+				      location_cookie);
+	size_t size = spa->header.length;
+
+	if (size == sizeof(*spa))
+		return size;
+
+	if (size != legacy_size)
+		return 0;
+
 	if (spa->flags & ACPI_NFIT_LOCATION_COOKIE_VALID)
-		return sizeof(*spa);
-	return sizeof(*spa) - 8;
+		return 0;
+
+	return size;
 }
 
 static bool add_spa(struct acpi_nfit_desc *acpi_desc,
@@ -716,23 +727,26 @@ static bool add_spa(struct acpi_nfit_desc *acpi_desc,
 {
 	struct device *dev = acpi_desc->dev;
 	struct nfit_spa *nfit_spa;
+	size_t size = sizeof_spa(spa);
 
-	if (spa->header.length != sizeof_spa(spa))
+	if (!size)
 		return false;
 
 	list_for_each_entry(nfit_spa, &prev->spas, list) {
-		if (memcmp(nfit_spa->spa, spa, sizeof_spa(spa)) == 0) {
+		if (sizeof_spa(nfit_spa->spa) != size)
+			continue;
+
+		if (memcmp(nfit_spa->spa, spa, size) == 0) {
 			list_move_tail(&nfit_spa->list, &acpi_desc->spas);
 			return true;
 		}
 	}
 
-	nfit_spa = devm_kzalloc(dev, sizeof(*nfit_spa) + sizeof_spa(spa),
-			GFP_KERNEL);
+	nfit_spa = devm_kzalloc(dev, sizeof(*nfit_spa) + size, GFP_KERNEL);
 	if (!nfit_spa)
 		return false;
 	INIT_LIST_HEAD(&nfit_spa->list);
-	memcpy(nfit_spa->spa, spa, sizeof_spa(spa));
+	memcpy(nfit_spa->spa, spa, size);
 	list_add_tail(&nfit_spa->list, &acpi_desc->spas);
 	dev_dbg(dev, "spa index: %d type: %s\n",
 			spa->range_index,
-- 
2.50.1 (Apple Git-155)

[PATCH v2 3/5] ACPI: NFIT: validate interleave and flush array extents

[Pengpeng Hou]

Interleave and flush subtables carry a fixed header followed by arrays
whose element counts come from firmware. sizeof_idt() and sizeof_flush()
derive the copy size from those counts but do not require the result to
fit in the subtable's advertised length.

A short subtable with a large line_count or hint_count can consequently
make add_idt() or add_flush() copy beyond the subtable. The outer NFIT
extent check cannot catch this because the following subtable bytes are
still inside the enclosing ACPI table.

Use struct_size() for both variable arrays and reject a derived size that
exceeds the current subtable.

Fixes: b94d5230d06e ("libnvdimm, nfit: initial libnvdimm infrastructure and NFIT support")
Assisted-by: Codex:gpt-5
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/acpi/nfit/core.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index f68edfe64952..bf4ddc56494e 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -882,9 +882,16 @@ static bool add_bdw(struct acpi_nfit_desc *acpi_desc,
 
 static size_t sizeof_idt(struct acpi_nfit_interleave *idt)
 {
+	size_t size;
+
 	if (idt->header.length < sizeof(*idt))
 		return 0;
-	return sizeof(*idt) + sizeof(u32) * idt->line_count;
+
+	size = struct_size(idt, line_offset, idt->line_count);
+	if (size > idt->header.length)
+		return 0;
+
+	return size;
 }
 
 static bool add_idt(struct acpi_nfit_desc *acpi_desc,
@@ -921,9 +928,16 @@ static bool add_idt(struct acpi_nfit_desc *acpi_desc,
 
 static size_t sizeof_flush(struct acpi_nfit_flush_address *flush)
 {
+	size_t size;
+
 	if (flush->header.length < sizeof(*flush))
 		return 0;
-	return struct_size(flush, hint_address, flush->hint_count);
+
+	size = struct_size(flush, hint_address, flush->hint_count);
+	if (size > flush->header.length)
+		return 0;
+
+	return size;
 }
 
 static bool add_flush(struct acpi_nfit_desc *acpi_desc,
-- 
2.50.1 (Apple Git-155)

[PATCH v2 4/5] ACPI: NFIT: validate control region extent

[Pengpeng Hou]

A control region with no block windows may legally stop before the
window-size fields, so sizeof_dcr() first reads the windows count from
the shortened fixed portion. When the count is nonzero, however, the
helper returns sizeof(*dcr) without checking that the advertised
subtable contains the remaining fields.

add_dcr() then copies that full size. A malformed subtable that ends
immediately after a nonzero windows count can therefore make the copy
read beyond the control region.

Compute the required size from the windows count and require that size
to fit in the advertised subtable before returning it.

Fixes: b94d5230d06e ("libnvdimm, nfit: initial libnvdimm infrastructure and NFIT support")
Assisted-by: Codex:gpt-5
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/acpi/nfit/core.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index bf4ddc56494e..84c70f1941c5 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -817,12 +817,20 @@ EXPORT_SYMBOL_GPL(nfit_get_smbios_id);
  */
 static size_t sizeof_dcr(struct acpi_nfit_control_region *dcr)
 {
+	size_t size;
+
 	if (dcr->header.length < offsetof(struct acpi_nfit_control_region,
 				window_size))
 		return 0;
 	if (dcr->windows)
-		return sizeof(*dcr);
-	return offsetof(struct acpi_nfit_control_region, window_size);
+		size = sizeof(*dcr);
+	else
+		size = offsetof(struct acpi_nfit_control_region, window_size);
+
+	if (size > dcr->header.length)
+		return 0;
+
+	return size;
 }
 
 static bool add_dcr(struct acpi_nfit_desc *acpi_desc,
-- 
2.50.1 (Apple Git-155)

[PATCH v2 5/5] ACPI: NFIT: bound the platform capability mask

[Pengpeng Hou]

The platform capability subtable parser reads highest_capability and
capabilities without first requiring the fixed structure to be present.
It also shifts a signed integer by highest_capability + 1. Firmware can
therefore trigger an out-of-bounds read with a short subtable and an
undefined shift with a value at or above 31.

Require the fixed structure before reading either field. For capability
indices that cover all bits representable by the current u32 field, use
U32_MAX. This ignores capabilities beyond the implemented 32-bit field
rather than rejecting future firmware that advertises a higher index.

Fixes: 06e8ccdab15f ("acpi: nfit: Add support for detect platform CPU cache flush on power loss")
Assisted-by: Codex:gpt-5
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/acpi/nfit/core.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 84c70f1941c5..7638d4dc8c6d 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -987,7 +987,14 @@ static bool add_platform_cap(struct acpi_nfit_desc *acpi_desc,
 	struct device *dev = acpi_desc->dev;
 	u32 mask;
 
-	mask = (1 << (pcap->highest_capability + 1)) - 1;
+	if (pcap->header.length < sizeof(*pcap))
+		return false;
+
+	if (pcap->highest_capability >= 31)
+		mask = U32_MAX;
+	else
+		mask = (1U << (pcap->highest_capability + 1)) - 1;
+
 	acpi_desc->platform_cap = pcap->capabilities & mask;
 	dev_dbg(dev, "cap: %#x\n", acpi_desc->platform_cap);
 	return true;
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2 0/5] ACPI: NFIT: harden subtable extent validation

[Alison Schofield]

On Thu, Aug 13, 2026 at 11:09:48PM +0800, Pengpeng Hou wrote:
> NFIT parsing has several independent extent contracts: the outer table
> must contain each advertised subtable, and variable or versioned records
> must fit inside that subtable.  The original patch mixed those contracts
> and also derived the SPA structure length from the location-cookie-valid
> flag.
> 
> Split the checks by record type.  Patch 1 validates the generic subtable
> header and outer extent.  Patch 2 accepts both defined SPA layouts while
> treating the cookie flag as a validity bit, not a size selector.  Patches
> 3 and 4 validate the variable arrays and the control-region extent.
> Patch 5 bounds the 32-bit capability mask without rejecting future
> firmware that advertises a higher capability index.
> 
> Changes since v1:
> https://lore.kernel.org/all/20260722041701.21078-1-pengpeng@iscas.ac.cn/

NAK
See: Documentation/process/submitting-patches.rst 
Section "Respond to review comments"


> - split the generic and type-specific contracts into separate patches
> - handle both 56-byte and 64-byte SPA structures
> - add the control-region extent check identified during review
> - clamp the implemented capability mask instead of rejecting future bits
> - describe the exact fields protected by each check
> 
> The series was reviewed statically against the current tree. I did not
> run an NFIT firmware fault-injection test.
> 
> Pengpeng Hou (5):
>   ACPI: NFIT: validate subtable extents before dispatch
>   ACPI: NFIT: accept SPA structures with an unused location cookie
>   ACPI: NFIT: validate interleave and flush array extents
>   ACPI: NFIT: validate control region extent
>   ACPI: NFIT: bound the platform capability mask
> 
>  drivers/acpi/nfit/core.c | 77 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 62 insertions(+), 15 deletions(-)
> 
> base-commit: f5bbbfec59b4e2fb7520a91de3df8a6174325d6a
> 
> -- 
> 2.50.1 (Apple Git-155)
>