linux-kernel - Re: [PATCH v3] irqchip/tango: Don't use incorrect irq_mask

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: <48734beb-0e6b-3a8f-ebf4-b1cec63322e5@gmail.com>
Date:   Thu, 27 Jul 2017 11:17:18 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Måns Rullgård <mans@...sr.com>,
        Marc Gonzalez <marc_gonzalez@...madesigns.com>,
        Mason <slash.tmp@...e.fr>
Cc:     Doug Berger <opendmb@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <marc.zyngier@....com>,
        Jason Cooper <jason@...edaemon.net>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3] irqchip/tango: Don't use incorrect irq_mask_ack
 callback

On 07/26/2017 12:13 PM, Måns Rullgård wrote:
> Florian Fainelli <f.fainelli@...il.com> writes:
> 
>> On 07/25/2017 06:29 AM, Måns Rullgård wrote:
>>> Marc Gonzalez <marc_gonzalez@...madesigns.com> writes:
>>>
>>>> On 25/07/2017 15:16, Måns Rullgård wrote:
>>>>
>>>>> What happened to the patch adding the proper combined function?
>>>>
>>>> It appears you're not CCed on v2.
>>>>
>>>> https://patchwork.kernel.org/patch/9859799/
>>>>
>>>> Doug wrote:
>>>>> Yes, you understand correctly.  The irq_mask_ack method is entirely
>>>>> optional and I assume that is why this issue went undetected for so
>>>>> long; however, it is slightly more efficient to combine the functions
>>>>> (even if the ack is unnecessary) which is why I chose to do so for my
>>>>> changes to the irqchip-brcmstb-l2 driver where I first discovered this
>>>>> issue.  How much value the improved efficiency has is certainly
>>>>> debatable, but interrupt handling is one area where people might care
>>>>> about such a small difference.  As the irqchip-tango driver maintainer
>>>>> you are welcome to decide whether or not the irq_mask_ack method makes
>>>>> sense to you.
>>>>
>>>> My preference goes to leaving the irq_mask_ack callback undefined,
>>>> and let the irqchip framework use irq_mask and irq_ack instead.
>>>
>>> Why would you prefer the less efficient way?
>>>
>>
>> Same question here, that does not really make sense to me.
>>
>> The whole point of this patch series is to have a set of efficient and
>> bugfree (or nearly) helper functions that drivers can rely on, are you
>> saying that somehow using irq_mask_and_ack is exposing a bug in the
>> tango irqchip driver and using the separate functions does not expose
>> this bug?
> 
> There is currently a bug in that the function used doesn't do what its
> name implies which can't be good.  Using the separate mask and ack
> functions obviously works, but combining them saves a lock/unlock
> sequence.  The correct combined function has already been written, so I
> see no reason not to use it.

Marc/Mason, are you intending to get this patch accepted in order to
provide a quick bugfix targeting earlier kernels with the tango irqchip
driver or is this how you think the correct fix for the tango irqchip
driver is as opposed to using Doug's fix?
-- 
Florian