[PATCH v2] lib/firmware_table: validate subtable bounds

[PATCH v2] lib/firmware_table: validate subtable bounds

[Jiale Yao]

The generic firmware table walker dispatches matching entries after only
checking that a generic subtable header is present. A record whose
declared length is shorter than its header or extends past the table can
therefore reach a handler before its bounds have been established.

Once a complete generic header is available, validate the declared record
range before looking up or invoking a handler. This rejects zero-length
and truncated records while preserving the existing behavior of ignoring
trailing table data too short to contain another subtable header.

Fixes: 60574d1e05b0 ("acpi: Create subtable parsing infrastructure")
Cc: stable@vger.kernel.org
Cc: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
Changes in v2:
- Preserve support for trailing padding shorter than a subtable header.
- Add the stable and code-author Cc trailers.
---
 lib/fw_table.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/lib/fw_table.c b/lib/fw_table.c
index 16291814450e..40846044e01b 100644
--- a/lib/fw_table.c
+++ b/lib/fw_table.c
@@ -172,7 +172,15 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
 	    ((unsigned long)table_header + table_size);
 	subtable_len = acpi_get_subtable_header_length(&entry);
 
-	while (((unsigned long)entry.hdr) + subtable_len < table_end) {
+	while ((unsigned long)entry.hdr + subtable_len <= table_end) {
+		entry_len = acpi_get_entry_length(&entry);
+		if (entry_len < subtable_len ||
+		    entry_len > table_end - (unsigned long)entry.hdr) {
+			pr_err("[%4.4s:0x%02lx] Invalid length %lu\n", id,
+			       acpi_get_entry_type(&entry), entry_len);
+			return -EINVAL;
+		}
+
 		for (i = 0; i < proc_num; i++) {
 			if (acpi_get_entry_type(&entry) != proc[i].id)
 				continue;
@@ -186,16 +194,6 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
 			break;
 		}
 
-		/*
-		 * If entry->length is 0, break from this loop to avoid
-		 * infinite loop.
-		 */
-		entry_len = acpi_get_entry_length(&entry);
-		if (entry_len == 0) {
-			pr_err("[%4.4s:0x%02x] Invalid zero length\n", id, proc->id);
-			return -EINVAL;
-		}
-
 		entry.hdr = (union acpi_subtable_headers *)
 		    ((unsigned long)entry.hdr + entry_len);
 	}
-- 
2.34.1

Re: [PATCH v2] lib/firmware_table: validate subtable bounds

[Andrew Morton]

On Wed, 21 Oct 2026 23:01:59 +0800 Jiale Yao <yaojiale02@163.com> wrote:

> The generic firmware table walker dispatches matching entries after only
> checking that a generic subtable header is present. A record whose
> declared length is shorter than its header or extends past the table can
> therefore reach a handler before its bounds have been established.

Then what happens?  Kernel go oops?

When fixing a bug please always describe the userspace-visible runtime
effects of that bug.

How can this situation come about?  A messed up firmware table?

> Once a complete generic header is available, validate the declared record
> range before looking up or invoking a handler. This rejects zero-length
> and truncated records while preserving the existing behavior of ignoring
> trailing table data too short to contain another subtable header.
>
> ...
>
> --- a/lib/fw_table.c
> +++ b/lib/fw_table.c
> @@ -172,7 +172,15 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
>  	    ((unsigned long)table_header + table_size);
>  	subtable_len = acpi_get_subtable_header_length(&entry);
>  
> -	while (((unsigned long)entry.hdr) + subtable_len < table_end) {
> +	while ((unsigned long)entry.hdr + subtable_len <= table_end) {
> +		entry_len = acpi_get_entry_length(&entry);
> +		if (entry_len < subtable_len ||
> +		    entry_len > table_end - (unsigned long)entry.hdr) {
> +			pr_err("[%4.4s:0x%02lx] Invalid length %lu\n", id,
> +			       acpi_get_entry_type(&entry), entry_len);
> +			return -EINVAL;
> +		}
> +
>  		for (i = 0; i < proc_num; i++) {
>  			if (acpi_get_entry_type(&entry) != proc[i].id)
>  				continue;
> @@ -186,16 +194,6 @@ acpi_parse_entries_array(char *id, unsigned long table_size,
>  			break;
>  		}
>  
> -		/*
> -		 * If entry->length is 0, break from this loop to avoid
> -		 * infinite loop.
> -		 */
> -		entry_len = acpi_get_entry_length(&entry);
> -		if (entry_len == 0) {
> -			pr_err("[%4.4s:0x%02x] Invalid zero length\n", id, proc->id);
> -			return -EINVAL;
> -		}
> -
>  		entry.hdr = (union acpi_subtable_headers *)
>  		    ((unsigned long)entry.hdr + entry_len);
>  	}
> -- 
> 2.34.1