linux-kernel - Re: [PATCH] irqchip/gic: Check interrupt type validity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 16 Nov 2019 13:35:08 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     linux-kernel@...r.kernel.org,
        bcm-kernel-feedback-list@...adcom.com,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>
Subject: Re: [PATCH] irqchip/gic: Check interrupt type validity

On Wed, 23 Oct 2019 12:56:19 -0700
Florian Fainelli <f.fainelli@...il.com> wrote:

> In case the interrupt property specifies a type parameter that is not
> GIC_SPI (0) or GIC_PPIC (1), do not attempt to translate the interrupt
> and return -EINVAL instead.
> 
> Fixes: f833f57ff254 ("irqchip: Convert all alloc/xlate users from of_node to fwnode")
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> ---
> Marc,
> 
> Regardless of whether my attempt to use SGI moves any further, this
> seems appropriate to do since we should not be trying to translate
> incorrectly specified interrupts. Thanks!
> 
>  drivers/irqchip/irq-gic.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 30ab623343d3..fc47e655618d 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1005,6 +1005,9 @@ static int gic_irq_domain_translate(struct irq_domain *d,
>  		if (fwspec->param_count < 3)
>  			return -EINVAL;
>  
> +		if (fwspec->param[0] > 1)
> +			return -EINVAL;
> +
>  		/* Get the interrupt number and add 16 to skip over SGIs */
>  		*hwirq = fwspec->param[1] + 16;
>  

I'm in two minds about this.

The usual stance is that the kernel is not a validation suite for DT
files, but on the other hand we already do some of that two lines above
(a consequence of kernel and DT binding lockstep development...). Do we
really want to add more of this? Or should we put more effort in static
validation of DT files and actually remove these checks?

	M.
-- 
Jazz is not dead. It just smells funny...