Re: [PATCH] irqdomain: check irq mapping against domain size

[Marc Zyngier <maz@kernel.org>]

Hi Ben,

On Fri, 05 Nov 2021 09:06:01 +0000,
Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> 
> The irq translate code does not check the irq number against
> the maximum a domain can handle. This can cause an OOPS if
> the firmware data has been damaged in any way. Check the intspec
> or fwdata against the irqdomain and return -EINVAL if over.
> 
> This is the result of bug somewhere in the boot of a SiFive Unmatched
> board where the 5th argument of the pcie node is being damaged which
> causes an OOPS in the startup code.
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  kernel/irq/irqdomain.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 6284443b87ec..e61397420723 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -906,6 +906,8 @@ int irq_domain_xlate_onecell(struct irq_domain *d, struct device_node *ctrlr,
>  {
>  	if (WARN_ON(intsize < 1))
>  		return -EINVAL;
> +	if (WARN_ON(intspec[0] > d->hwirq_max))
> +		return -EINVAL;

This doesn't seem right.

For a start, d->hwirq_max is 0 when the domain is backed by a radix
tree. Also, nothing says that what you read from the DT is something
that should be directly meaningful to the irqdomain. A driver could
well call into this and perform some extra processing on the data
before it lands into the irqdomain.

In general, this looks like DT validation code, and I'm not keen on
that in the core code.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.