Re: [PATCH] ACPI: IORT: validate RMR node array bounds

[Hanjun Guo <guohanjun@huawei.com>]

On 2026/7/6 17:43, Pengpeng Hou wrote:
> IORT RMR nodes describe reserved-memory ranges through firmware
> offset and count fields inside the current IORT node.
> 
> Validate the generic IORT node length before dispatching it, and
> check both the RMR descriptor array and the ID mapping array before
> walking them.  This binds each array walk to the current node length
> instead of only trusting the firmware-provided count.

Would you mind split this into two patches? one for iort_node_valid(),
the other is valid the IORT RMR nodes.

> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>   drivers/acpi/arm64/iort.c | 83 +++++++++++++++++++++++++++++++++++++--
>   1 file changed, 79 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index af7a9b2fd5bc..55373246bdd4 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -15,6 +15,7 @@
>   #include <linux/iommu.h>
>   #include <linux/kernel.h>
>   #include <linux/list.h>
> +#include <linux/overflow.h>
>   #include <linux/pci.h>
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
> @@ -148,6 +149,69 @@ typedef acpi_status (*iort_find_node_callback)
>   /* Root pointer to the mapped IORT table */
>   static struct acpi_table_header *iort_table;
>   
> +static bool iort_node_valid(struct acpi_iort_node *node,
> +			    struct acpi_iort_node *end)
> +{
> +	size_t remaining;
> +
> +	if (WARN_TAINT(node >= end, TAINT_FIRMWARE_WORKAROUND,
> +		       "IORT node pointer overflows, bad table!\n"))
> +		return false;
> +
> +	remaining = (char *)end - (char *)node;
> +	if (WARN_TAINT(remaining < sizeof(*node), TAINT_FIRMWARE_WORKAROUND,
> +		       "IORT node header is truncated, bad table!\n"))
> +		return false;
> +
> +	if (WARN_TAINT(node->length < sizeof(*node) || node->length > remaining,

Check the node length as well.

> +		       TAINT_FIRMWARE_WORKAROUND,
> +		       "IORT node length overflows, bad table!\n"))
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool iort_node_array_valid(struct acpi_iort_node *node, u32 offset,
> +				  u32 count, size_t elem_size,
> +				  size_t min_offset, const char *name)
> +{
> +	size_t bytes;
> +
> +	if (!count)
> +		return true;

Please add a comment here why count = 0 is valid.

> +
> +	if (!offset || offset < min_offset || offset > node->length) {
> +		pr_err(FW_BUG "Invalid %s offset in IORT node %p\n", name,
> +		       node);
> +		return false;
> +	}
> +
> +	if (check_mul_overflow(count, elem_size, &bytes) ||
> +	    bytes > node->length - offset) {
> +		pr_err(FW_BUG "Invalid %s array in IORT node %p\n", name,
> +		       node);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static bool iort_rmr_node_valid(struct acpi_iort_node *node)
> +{
> +	struct acpi_iort_rmr *rmr;
> +
> +	if (node->length < sizeof(*node) + sizeof(*rmr)) {
> +		pr_err(FW_BUG "Truncated RMR node in IORT table\n");
> +		return false;
> +	}
> +
> +	rmr = (struct acpi_iort_rmr *)node->node_data;
> +	return iort_node_array_valid(node, rmr->rmr_offset, rmr->rmr_count,
> +				     sizeof(struct acpi_iort_rmr_desc),
> +				     sizeof(*node) + sizeof(*rmr),
> +				     "RMR descriptor");
> +}
> +
>   static LIST_HEAD(iort_msi_chip_list);
>   static DEFINE_SPINLOCK(iort_msi_chip_lock);
>   
> @@ -243,8 +307,7 @@ static struct acpi_iort_node *iort_scan_node(enum acpi_iort_node_type type,
>   				iort_table->length);
>   
>   	for (i = 0; i < iort->node_count; i++) {
> -		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
> -			       "IORT node pointer overflows, bad table!\n"))
> +		if (!iort_node_valid(iort_node, iort_end))
>   			return NULL;
>   
>   		if (iort_node->type == type &&
> @@ -1014,6 +1077,9 @@ static void iort_get_rmrs(struct acpi_iort_node *node,
>   	struct acpi_iort_rmr_desc *rmr_desc;
>   	int i;
>   
> +	if (!iort_rmr_node_valid(node))
> +		return;
> +
>   	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, node,
>   				rmr->rmr_offset);
>   
> @@ -1114,6 +1180,16 @@ static void iort_node_get_rmr_info(struct acpi_iort_node *node,
>   		return;
>   	}
>   
> +	if (!iort_node_array_valid(node, node->mapping_offset,
> +				   node->mapping_count,
> +				   sizeof(struct acpi_iort_id_mapping),
> +				   sizeof(*node),
> +				   "ID mapping"))
> +		return;
> +
> +	if (!iort_rmr_node_valid(node))
> +		return;
> +
>   	rmr = (struct acpi_iort_rmr *)node->node_data;
>   	if (!rmr->rmr_offset || !rmr->rmr_count)
>   		return;
> @@ -1175,8 +1251,7 @@ static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,
>   				iort_table->length);
>   
>   	for (i = 0; i < iort->node_count; i++) {
> -		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
> -			       "IORT node pointer overflows, bad table!\n"))
> +		if (!iort_node_valid(iort_node, iort_end))
>   			return;
>   
>   		if (iort_node->type == ACPI_IORT_NODE_RMR)

Thanks
Hanjun