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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 10 Dec 2019 12:25:07 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Marc Zyngier <maz@...nel.org>,
        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 11/16/19 5:35 AM, Marc Zyngier wrote:
> 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?


Static validation is nice for DTS that live in the tree, if you have a
platform that provides the FDT via firmware, and that firmware is handed
over to a variety of people, who knows what they can do. My inclination
would be to perform that check which is not costly and guarantees we
don't feed garbage to the GIC driver.
-- 
Florian