linux-kernel - Re: [PATCH 1/2] irqdomain: fix irq_domain_xlate

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]

Message-ID: <57A31994.7030503@laposte.net>
Date:	Thu, 4 Aug 2016 12:31:48 +0200
From:	Sebastian Frias <sf84@...oste.net>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Grant Likely <grant.likely@...retlab.ca>,
	Marc Zyngier <marc.zyngier@....com>,
	Jason Cooper <jason@...edaemon.net>,
	LKML <linux-kernel@...r.kernel.org>, Mason <slash.tmp@...e.fr>
Subject: Re: [PATCH 1/2] irqdomain: fix irq_domain_xlate_onetwocell()

Hi Thomas,

On 08/04/2016 12:05 PM, Thomas Gleixner wrote:
> On Tue, 2 Aug 2016, Sebastian Frias wrote:
> 
>> Use IRQ_TYPE_SENSE_MASK on irq_domain_xlate_onetwocell()
>>
>> According to the xlate() callback definition, the 'out_type' parameter
>> needs to be the "linux irq type". A mask for such bits exists,
>> IRQ_TYPE_SENSE_MASK, and as a matter of fact it is used in
>> irq_domain_xlate_twocell(), use it for irq_domain_xlate_onetwocell() as
>> well.
>>
>> Fixes: 16b2e6e2f31d ("irq_domain: Create common xlate functions that device
>> drivers can use")
> 
> You seem to be obsessed by adding "Fixes" tags and 'Fix' to the $subject, but
> your changelog just babbles about making the code the same as in
> irq_domain_xlate_twocell() and avoids carefully to explain what is broken,
> what consequences that has and what the fix is.

I'm sorry but I think your comment above is unfair, so you could have omitted
it entirely.

> 
> Now if you look at the call site which uses the xlate functions then you'll
> notice that we have a sanity check there already:
> 
>         if (irq_domain_translate(domain, fwspec, &hwirq, &type))
>                 return 0;
>         /*
>          * WARN if the irqchip returns a type with bits
>          * outside the sense mask set and clear these bits.
>          */
>         if (WARN_ON(type & ~IRQ_TYPE_SENSE_MASK))
>                 type &= IRQ_TYPE_SENSE_MASK;
> 

(for what is worth, my patch was based on a recent 4.7 that does not has the
code you mention above)

> So we better remove the masking in the xlate functions completely and let all
> the offending device trees trip over the warning so they get eventually fixed.

That's fine by me.
If you want I can submit a patch that does what you want (if so, should I use
"fixes" or not?), let me know.

Best regards,

Sebastian