linux-kernel - Re: [PATCH 2/2] pinctrl/amd: use byte access to clear irq/wake status bits

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Jul 2018 07:30:22 -0500
From:   Daniel Drake <drake@...lessm.com>
To:     Daniel Kurtz <djkurtz@...omium.org>
Cc:     Shyam Sundar S K <Shyam-sundar.S-k@....com>,
        Nehal Shah <Nehal-bakulchandra.Shah@....com>,
        Ken Xue <Ken.Xue@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        "open list:PIN CONTROL SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] pinctrl/amd: use byte access to clear irq/wake status bits

On Mon, Jul 16, 2018 at 7:57 PM, Daniel Kurtz <djkurtz@...omium.org> wrote:
> Commit 6afb10267c1692 ("pinctrl/amd: fix masking of GPIO interrupts")
> changed to the clearing of interrupt status bits to a RMW in a critical
> section.  This works, but is a bit overkill.
>
> The relevant interrupt/wake status bits are in the Most Significant Byte
> of a 32-bit word.  These two are the only write-able bits in this byte.

I don't have the hardware to test this any more, and I also don't have
any docs to double if those are really the only writable bits, but
looking at the existing driver code it does seem to be the case.

I think you should retain the comment noting that the value of the
register may have changed since it was read just a few lines above
(and hence explaining more precisely why we make the special effort
just to modify the MSB), just in case there is further rework of this
code in future and we end up walking into the same trap. It was one of
those issues that took a frustratingly long time to figure out...

Thanks
Daniel